[Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Fam Zheng]

CURL library API has changed, the current curl driver is not working.
This patch rewrites the use of API as well as the structure of internal
states. 

BDRVCURLState holds the pointer to curl multi interface (man 3
libcurl-multi), and 4 lists for internal states:
 - CURLState holds state for libcurl connection (man 3 libcurl-easy)
 - CURLSockInfo holds information for libcurl socket interface (man 3
   curl_multi_socket_action).
 - CURLDataCache holds the user data read from libcurl, it is in a list
   ordered by access, the used cache is moved to list head on access, so
   the tail element is freed first. BDRVCURLState.cache_quota is the
   threshold to start freeing cache.
 - CURLAIOCB holds ongoing aio information.

Changes from v3:
  01, 06, 07: Add QLIST_INIT in qemu_open to initialize each list.
  07: Move clean up for s->acbs from later patch to here. Use qemu_aio_relase instead of g_free on acb.
      Fix use-after-free bug. [Rich]

Changes from v2:
  00: Removed previous 09, since changes are included in previous
      commits.
      previous 09: curl: release everything on curl close.
  01: Add commit message for introducing CURLSockInfo. Remove
      unnecessary pointer type cast.
  02: Use sizeof instead of macro for s->range len.
  04: Use pstrcpy and strncasecmp.
  06: Add commit message for why is there a cache in curl.c.

Changes from v1:
   Split to multiple patches. The final status should be identical to v1.

Fam Zheng (10):
  curl: introduce CURLSockInfo to BDRVCURLState.
  curl: change magic number to sizeof
  curl: change curl_multi_do to curl_fd_handler
  curl: fix curl_open
  curl: add timer to BDRVCURLState
  curl: introduce CURLDataCache
  curl: make use of CURLDataCache.
  curl: use list to store CURLState
  curl: add cache quota.
  curl: introduce ssl_no_cert runtime option.

 block/curl.c | 574 +++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 361 insertions(+), 213 deletions(-)

-- 
1.8.2.3

[Qemu-devel] [PATCH v4 01/10] curl: introduce CURLSockInfo to BDRVCURLState.

[Fam Zheng]

We use socket provided by curl in the driver.  Libcurl multi interface
has option CURLMOPT_SOCKETFUNCTION for socket.

Per man 3 curl_multi_setopt:

    ...
    CURLMOPT_SOCKETFUNCTION

    Pass a pointer to a function matching the curl_socket_callback
    prototype. The curl_multi_socket_action(3) function informs the
    application about updates in the socket (file descriptor) status by
    doing none, one, or multiple calls to the curl_socket_callback given in
    the param argument. They update the status with changes since the
    previous time a curl_multi_socket(3) function was called. If the given
    callback pointer is NULL, no callback will be called. Set the callback's
    userp argument with CURLMOPT_SOCKETDATA. See curl_multi_socket(3) for
    more callback details.
    ...

The added structure store information for all the socket returned by
libcurl. The most important field is action, which is used to keep what
actions are needed when this socket's fd handler is called.

CURLSockInfo is added here and used in later commits to save the socket
actions.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index b8935fd..2e42cd1 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -75,10 +75,18 @@ typedef struct CURLState
     char in_use;
 } CURLState;
 
+typedef struct CURLSockInfo {
+    curl_socket_t fd;
+    int action;
+    struct BDRVCURLState *s;
+    QLIST_ENTRY(CURLSockInfo) next;
+} CURLSockInfo;
+
 typedef struct BDRVCURLState {
     CURLM *multi;
     size_t len;
     CURLState states[CURL_NUM_STATES];
+    QLIST_HEAD(, CURLSockInfo) socks;
     char *url;
     size_t readahead_size;
 } BDRVCURLState;
@@ -90,7 +98,16 @@ static int curl_aio_flush(void *opaque);
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
                         void *s, void *sp)
 {
+    BDRVCURLState *bs = s;
     DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, fd);
+    CURLSockInfo *sock = sp;
+    if (!sp) {
+        sock = g_malloc0(sizeof(CURLSockInfo));
+        sock->fd = fd;
+        sock->s = bs;
+        QLIST_INSERT_HEAD(&bs->socks, sock, next);
+        curl_multi_assign(bs->multi, fd, sock);
+    }
     switch (action) {
         case CURL_POLL_IN:
             qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, curl_aio_flush, s);
@@ -433,6 +450,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
         inited = 1;
     }
 
+    QLIST_INIT(&s->socks);
+
     DPRINTF("CURL: Opening %s\n", file);
     s->url = g_strdup(file);
     state = curl_init_state(s);
@@ -462,8 +481,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     // 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 ); 
+    curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
+    curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
     curl_multi_do(s);
 
     qemu_opts_del(opts);
@@ -603,6 +622,14 @@ static void curl_close(BlockDriverState *bs)
     }
     if (s->multi)
         curl_multi_cleanup(s->multi);
+
+    while (!QLIST_EMPTY(&s->socks)) {
+        CURLSockInfo *sock = QLIST_FIRST(&s->socks);
+        QLIST_REMOVE(sock, next);
+        g_free(sock);
+        sock = NULL;
+    }
+
     g_free(s->url);
 }
 
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 02/10] curl: change magic number to sizeof

[Fam Zheng]

String field length is duplicated in two places. Make it a sizeof.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 2e42cd1..d0bc66f 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -569,7 +569,7 @@ static void curl_readv_bh_cb(void *p)
     state->orig_buf = g_malloc(state->buf_len);
     state->acb[0] = acb;
 
-    snprintf(state->range, 127, "%zd-%zd", start, end);
+    snprintf(state->range, sizeof(state->range) - 1, "%zd-%zd", start, end);
     DPRINTF("CURL (AIO): Reading %d at %zd (%s)\n",
             (acb->nb_sectors * SECTOR_SIZE), start, state->range);
     curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 03/10] curl: change curl_multi_do to curl_fd_handler

[Fam Zheng]

The driver calls curl_multi_do to take action at several points, while
it's also registered as socket fd handler. This patch removes internal
call of curl_multi_do because they are not necessary when handler can be
called by socket data update.

Since curl_multi_do becomes a pure fd handler, the function is renamed.
It takes a pointer to CURLSockInfo now, instead of pointer to
BDRVCURLState.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index d0bc66f..4a56cfd 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -92,7 +92,7 @@ typedef struct BDRVCURLState {
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
-static void curl_multi_do(void *arg);
+static void curl_fd_handler(void *arg);
 static int curl_aio_flush(void *opaque);
 
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
@@ -110,17 +110,23 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
     }
     switch (action) {
         case CURL_POLL_IN:
-            qemu_aio_set_fd_handler(fd, curl_multi_do, NULL, curl_aio_flush, s);
+            qemu_aio_set_fd_handler(fd, curl_fd_handler, NULL,
+                                    curl_aio_flush, sock);
+            sock->action |= CURL_CSELECT_IN;
             break;
         case CURL_POLL_OUT:
-            qemu_aio_set_fd_handler(fd, NULL, curl_multi_do, curl_aio_flush, s);
+            qemu_aio_set_fd_handler(fd, NULL, curl_fd_handler, curl_aio_flush,
+                                    sock);
+            sock->action |= CURL_CSELECT_OUT;
             break;
         case CURL_POLL_INOUT:
-            qemu_aio_set_fd_handler(fd, curl_multi_do, curl_multi_do,
-                                    curl_aio_flush, s);
+            qemu_aio_set_fd_handler(fd, curl_fd_handler, curl_fd_handler,
+                                    curl_aio_flush, sock);
+            sock->action |= CURL_CSELECT_IN | CURL_CSELECT_OUT;
             break;
         case CURL_POLL_REMOVE:
             qemu_aio_set_fd_handler(fd, NULL, NULL, NULL, NULL);
+            sock->action = 0;
             break;
     }
 
@@ -226,9 +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_fd_handler(void *arg)
 {
-    BDRVCURLState *s = (BDRVCURLState *)arg;
+    CURLSockInfo *sock = (CURLSockInfo *)arg;
+    BDRVCURLState *s = sock->s;
     int running;
     int r;
     int msgs_in_queue;
@@ -237,7 +244,9 @@ static void curl_multi_do(void *arg)
         return;
 
     do {
-        r = curl_multi_socket_all(s->multi, &running);
+        r = curl_multi_socket_action(s->multi,
+                sock->fd, sock->action,
+                &running);
     } while(r == CURLM_CALL_MULTI_PERFORM);
 
     /* Try to find done transfers, so we can free the easy
@@ -302,7 +311,6 @@ static CURLState *curl_init_state(BDRVCURLState *s)
         }
         if (!state) {
             g_usleep(100);
-            curl_multi_do(s);
         }
     } while(!state);
 
@@ -483,7 +491,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     s->multi = curl_multi_init();
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
-    curl_multi_do(s);
 
     qemu_opts_del(opts);
     return 0;
@@ -575,7 +582,6 @@ static void curl_readv_bh_cb(void *p)
     curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
 
     curl_multi_add_handle(s->multi, state->curl);
-    curl_multi_do(s);
 
 }
 
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 04/10] curl: fix curl_open

[Fam Zheng]

Change curl_size_cb to curl_header_cb, as what the function is really
doing. Fix the registering, CURLOPT_WRITEFUNCTION is apparently wrong,
should be CURLOPT_HEADERFUNCTION.

Parsing size from header is not necessary as we're using

  curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d)

The validity of d is version dependant per man 3 curl_easy_getinfo:

    ...
    CURLINFO_CONTENT_LENGTH_DOWNLOAD

    Pass a pointer to a double to receive the content-length of the
    download. This is the value read from the Content-Length: field.
    Since 7.19.4, this returns -1 if the size isn't known.
    ...

And Accept-Ranges is essential for curl to fetch right data from
http/https servers, so we check for this in the header and fail the open
if it's not supported.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 4a56cfd..fc464ad 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -89,6 +89,8 @@ typedef struct BDRVCURLState {
     QLIST_HEAD(, CURLSockInfo) socks;
     char *url;
     size_t readahead_size;
+    /* Whether http server accept range in header */
+    bool accept_range;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -133,14 +135,14 @@ static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
     return 0;
 }
 
-static size_t curl_size_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
+static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
-    CURLState *s = ((CURLState*)opaque);
+    BDRVCURLState *s = (BDRVCURLState *)opaque;
     size_t realsize = size * nmemb;
-    size_t fsize;
+    const char *accept_line = "Accept-Ranges: bytes";
 
-    if(sscanf(ptr, "Content-Length: %zd", &fsize) == 1) {
-        s->s->len = fsize;
+    if (strncmp((char *)ptr, accept_line, strlen(accept_line)) == 0) {
+        s->accept_range = true;
     }
 
     return realsize;
@@ -428,6 +430,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     Error *local_err = NULL;
     const char *file;
     double d;
+    int running;
 
     static int inited = 0;
 
@@ -469,16 +472,29 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     // Get file size
 
     curl_easy_setopt(state->curl, CURLOPT_NOBODY, 1);
-    curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_size_cb);
-    if (curl_easy_perform(state->curl))
+    curl_easy_setopt(state->curl, CURLOPT_HEADERFUNCTION,
+                     curl_header_cb);
+    curl_easy_setopt(state->curl, CURLOPT_HEADERDATA, s);
+    if (curl_easy_perform(state->curl)) {
         goto out;
+    }
     curl_easy_getinfo(state->curl, CURLINFO_CONTENT_LENGTH_DOWNLOAD, &d);
-    curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb);
     curl_easy_setopt(state->curl, CURLOPT_NOBODY, 0);
-    if (d)
+#if LIBCURL_VERSION_NUM > 0x071304
+    if (d != -1) {
+#else
+    if (d) {
+#endif
         s->len = (size_t)d;
-    else if(!s->len)
+    } else if (!s->len) {
+        goto out;
+    }
+    if (!strncasecmp(s->url, "http://", strlen("http://"))
+        && !strncasecmp(s->url, "https://", strlen("https://"))
+        && !s->accept_range) {
+        pstrcpy(state->errmsg, CURL_ERROR_SIZE, "Server not supporting range.");
         goto out;
+    }
     DPRINTF("CURL: Size = %zd\n", s->len);
 
     curl_clean_state(state);
@@ -487,10 +503,13 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
 
     // Now we know the file exists and its size, so let's
     // initialize the multi interface!
-
     s->multi = curl_multi_init();
+    if (!s->multi) {
+        goto out_noclean;
+    }
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
+    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
 
     qemu_opts_del(opts);
     return 0;
@@ -500,8 +519,9 @@ out:
     curl_easy_cleanup(state->curl);
     state->curl = NULL;
 out_noclean:
-    g_free(s->url);
     qemu_opts_del(opts);
+    g_free(s->url);
+    s->url = NULL;
     return -EINVAL;
 }
 
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 05/10] curl: add timer to BDRVCURLState

[Fam Zheng]

libcurl uses timer to manage ongoing sockets, it needs us to supply
timer. This patch introduce QEMUTimer to BDRVCURLState and handles
timeouts as libcurl expects (curl_multi_timer_cb sets given timeout
value on the timer and curl_timer_cb calls curl_multi_socket_action on
triggered).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index fc464ad..4fd5bb9 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -89,6 +89,7 @@ typedef struct BDRVCURLState {
     QLIST_HEAD(, CURLSockInfo) socks;
     char *url;
     size_t readahead_size;
+    QEMUTimer *timer;
     /* Whether http server accept range in header */
     bool accept_range;
 } BDRVCURLState;
@@ -148,6 +149,38 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
     return realsize;
 }
 
+static void curl_timer_cb(void *opaque)
+{
+    int running;
+    BDRVCURLState *bs = (BDRVCURLState *)opaque;
+    DPRINTF("curl timeout!\n");
+    curl_multi_socket_action(bs->multi, CURL_SOCKET_TIMEOUT, 0, &running);
+}
+
+/* Call back for curl_multi interface */
+static int curl_multi_timer_cb(CURLM *multi, long timeout_ms, void *s)
+{
+    BDRVCURLState *bs = (BDRVCURLState *)s;
+    DPRINTF("curl multi timer cb, timeout: %ld (ms)\n", timeout_ms);
+    if (timeout_ms < 0) {
+        if (bs->timer) {
+            qemu_del_timer(bs->timer);
+            qemu_free_timer(bs->timer);
+            bs->timer = NULL;
+        }
+    } else if (timeout_ms == 0) {
+        curl_timer_cb(bs);
+    } else {
+        if (!bs->timer) {
+            bs->timer = qemu_new_timer_ms(host_clock, curl_timer_cb, s);
+            assert(bs->timer);
+        }
+        qemu_mod_timer(bs->timer, qemu_get_clock_ms(host_clock) + timeout_ms);
+    }
+
+    return 0;
+}
+
 static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
     CURLState *s = ((CURLState*)opaque);
@@ -509,6 +542,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     }
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
+    curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_multi_timer_cb);
     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
 
     qemu_opts_del(opts);
@@ -634,6 +669,13 @@ static void curl_close(BlockDriverState *bs)
     int i;
 
     DPRINTF("CURL: Close\n");
+
+    if (s->timer) {
+        qemu_del_timer(s->timer);
+        qemu_free_timer(s->timer);
+        s->timer = NULL;
+    }
+
     for (i=0; i<CURL_NUM_STATES; i++) {
         if (s->states[i].in_use)
             curl_clean_state(&s->states[i]);
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 06/10] curl: introduce CURLDataCache

[Fam Zheng]

Data buffer was contained by CURLState, they are allocated and freed
together. This patch try to isolate them, by introducing a dedicated
cache list to BDRVCURLState. The benifit is we can now release the
CURLState (and associated sockets) while keep the fetched data for later
use, and simplies the prefetch and buffer logic for some degree.

Note: There's already page cache in guest kernel, why cache data here?
Since we don't want to submit http/ftp/* request for every 2KB in
sequencial read, but there are crude guest that sends small IO reqs,
which will result in horrible performance.  GRUB/isolinux loading kernel
is a typical case and we workaround this by prefetch cache.  This is
what curl.c has been doing along. This patch just refectors the buffer.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 136 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 67 insertions(+), 69 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 4fd5bb9..e387ae1 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -43,10 +43,6 @@
 #define SECTOR_SIZE     512
 #define READ_AHEAD_SIZE (256 * 1024)
 
-#define FIND_RET_NONE   0
-#define FIND_RET_OK     1
-#define FIND_RET_WAIT   2
-
 struct BDRVCURLState;
 
 typedef struct CURLAIOCB {
@@ -61,6 +57,16 @@ typedef struct CURLAIOCB {
     size_t end;
 } CURLAIOCB;
 
+typedef struct CURLDataCache {
+    char *data;
+    size_t base_pos;
+    size_t data_len;
+    size_t write_pos;
+    /* Ref count for CURLState */
+    int use_count;
+    QLIST_ENTRY(CURLDataCache) next;
+} CURLDataCache;
+
 typedef struct CURLState
 {
     struct BDRVCURLState *s;
@@ -90,6 +96,8 @@ typedef struct BDRVCURLState {
     char *url;
     size_t readahead_size;
     QEMUTimer *timer;
+    /* List of data cache ordered by access, freed from tail */
+    QLIST_HEAD(, CURLDataCache) cache;
     /* Whether http server accept range in header */
     bool accept_range;
 } BDRVCURLState;
@@ -98,6 +106,19 @@ static void curl_clean_state(CURLState *s);
 static void curl_fd_handler(void *arg);
 static int curl_aio_flush(void *opaque);
 
+static CURLDataCache *curl_find_cache(BDRVCURLState *bs,
+                                      size_t start, size_t len)
+{
+    CURLDataCache *c;
+    QLIST_FOREACH(c, &bs->cache, next) {
+        if (start >= c->base_pos &&
+            start + len <= c->base_pos + c->write_pos) {
+            return c;
+        }
+    }
+    return NULL;
+}
+
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
                         void *s, void *sp)
 {
@@ -181,6 +202,23 @@ static int curl_multi_timer_cb(CURLM *multi, long timeout_ms, void *s)
     return 0;
 }
 
+static void curl_complete_io(BDRVCURLState *bs, CURLAIOCB *acb,
+                             CURLDataCache *cache)
+{
+    size_t aio_base = acb->sector_num * SECTOR_SIZE;
+    size_t aio_bytes = acb->nb_sectors * SECTOR_SIZE;
+    size_t off = aio_base - cache->base_pos;
+
+    qemu_iovec_from_buf(acb->qiov, 0, cache->data + off, aio_bytes);
+    acb->common.cb(acb->common.opaque, 0);
+    DPRINTF("AIO Request OK: %10zd %10zd\n", aio_base, aio_bytes);
+    qemu_aio_release(acb);
+    acb = NULL;
+    /* Move cache next in the list */
+    QLIST_REMOVE(cache, next);
+    QLIST_INSERT_HEAD(&bs->cache, cache, next);
+}
+
 static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
     CURLState *s = ((CURLState*)opaque);
@@ -214,59 +252,6 @@ read_end:
     return realsize;
 }
 
-static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,
-                         CURLAIOCB *acb)
-{
-    int i;
-    size_t end = start + len;
-
-    for (i=0; i<CURL_NUM_STATES; i++) {
-        CURLState *state = &s->states[i];
-        size_t buf_end = (state->buf_start + state->buf_off);
-        size_t buf_fend = (state->buf_start + state->buf_len);
-
-        if (!state->orig_buf)
-            continue;
-        if (!state->buf_off)
-            continue;
-
-        // Does the existing buffer cover our section?
-        if ((start >= state->buf_start) &&
-            (start <= buf_end) &&
-            (end >= state->buf_start) &&
-            (end <= buf_end))
-        {
-            char *buf = state->orig_buf + (start - state->buf_start);
-
-            qemu_iovec_from_buf(acb->qiov, 0, buf, len);
-            acb->common.cb(acb->common.opaque, 0);
-
-            return FIND_RET_OK;
-        }
-
-        // Wait for unfinished chunks
-        if ((start >= state->buf_start) &&
-            (start <= buf_fend) &&
-            (end >= state->buf_start) &&
-            (end <= buf_fend))
-        {
-            int j;
-
-            acb->start = start - state->buf_start;
-            acb->end = acb->start + len;
-
-            for (j=0; j<CURL_NUM_ACB; j++) {
-                if (!state->acb[j]) {
-                    state->acb[j] = acb;
-                    return FIND_RET_WAIT;
-                }
-            }
-        }
-    }
-
-    return FIND_RET_NONE;
-}
-
 static void curl_fd_handler(void *arg)
 {
     CURLSockInfo *sock = (CURLSockInfo *)arg;
@@ -299,7 +284,9 @@ static void curl_fd_handler(void *arg)
             case CURLMSG_DONE:
             {
                 CURLState *state = NULL;
-                curl_easy_getinfo(msg->easy_handle, CURLINFO_PRIVATE, (char**)&state);
+                curl_easy_getinfo(msg->easy_handle,
+                                  CURLINFO_PRIVATE,
+                                  (char **)&state);
 
                 /* ACBs for successful messages get completed in curl_read_cb */
                 if (msg->data.result != CURLE_OK) {
@@ -495,6 +482,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     }
 
     QLIST_INIT(&s->socks);
+    QLIST_INIT(&s->cache);
 
     DPRINTF("CURL: Opening %s\n", file);
     s->url = g_strdup(file);
@@ -589,26 +577,24 @@ static const AIOCBInfo curl_aiocb_info = {
 static void curl_readv_bh_cb(void *p)
 {
     CURLState *state;
-
+    CURLDataCache *cache = NULL;
     CURLAIOCB *acb = p;
     BDRVCURLState *s = acb->common.bs->opaque;
+    size_t aio_base, aio_bytes;
 
     qemu_bh_delete(acb->bh);
     acb->bh = NULL;
 
+    aio_base = acb->sector_num * SECTOR_SIZE;
+    aio_bytes = acb->nb_sectors * SECTOR_SIZE;
+
     size_t start = acb->sector_num * SECTOR_SIZE;
     size_t end;
 
-    // In case we have the requested data already (e.g. read-ahead),
-    // we can just call the callback and be done.
-    switch (curl_find_buf(s, start, acb->nb_sectors * SECTOR_SIZE, acb)) {
-        case FIND_RET_OK:
-            qemu_aio_release(acb);
-            // fall through
-        case FIND_RET_WAIT:
-            return;
-        default:
-            break;
+    cache = curl_find_cache(s, aio_base, aio_bytes);
+    if (cache) {
+        curl_complete_io(s, acb, cache);
+        return;
     }
 
     // No cache found, so let's start a new request
@@ -691,6 +677,18 @@ static void curl_close(BlockDriverState *bs)
     if (s->multi)
         curl_multi_cleanup(s->multi);
 
+    while (!QLIST_EMPTY(&s->cache)) {
+        CURLDataCache *cache = QLIST_FIRST(&s->cache);
+        assert(cache->use_count == 0);
+        if (cache->data) {
+            g_free(cache->data);
+            cache->data = NULL;
+        }
+        QLIST_REMOVE(cache, next);
+        g_free(cache);
+        cache = NULL;
+    }
+
     while (!QLIST_EMPTY(&s->socks)) {
         CURLSockInfo *sock = QLIST_FIRST(&s->socks);
         QLIST_REMOVE(sock, next);
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 07/10] curl: make use of CURLDataCache.

[Fam Zheng]

Make subsequecial changes to make use of introduced CURLDataCache. Moved
acb struct from CURLState to BDRVCURLState, and changed to list.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 168 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 90 insertions(+), 78 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index e387ae1..4c4752b 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -39,7 +39,6 @@
                    CURLPROTO_TFTP)
 
 #define CURL_NUM_STATES 8
-#define CURL_NUM_ACB    8
 #define SECTOR_SIZE     512
 #define READ_AHEAD_SIZE (256 * 1024)
 
@@ -52,9 +51,7 @@ typedef struct CURLAIOCB {
 
     int64_t sector_num;
     int nb_sectors;
-
-    size_t start;
-    size_t end;
+    QLIST_ENTRY(CURLAIOCB) next;
 } CURLAIOCB;
 
 typedef struct CURLDataCache {
@@ -70,14 +67,10 @@ typedef struct CURLDataCache {
 typedef struct CURLState
 {
     struct BDRVCURLState *s;
-    CURLAIOCB *acb[CURL_NUM_ACB];
     CURL *curl;
-    char *orig_buf;
-    size_t buf_start;
-    size_t buf_off;
-    size_t buf_len;
     char range[128];
     char errmsg[CURL_ERROR_SIZE];
+    CURLDataCache *cache;
     char in_use;
 } CURLState;
 
@@ -92,6 +85,7 @@ typedef struct BDRVCURLState {
     CURLM *multi;
     size_t len;
     CURLState states[CURL_NUM_STATES];
+    QLIST_HEAD(, CURLAIOCB) acbs;
     QLIST_HEAD(, CURLSockInfo) socks;
     char *url;
     size_t readahead_size;
@@ -221,31 +215,35 @@ static void curl_complete_io(BDRVCURLState *bs, CURLAIOCB *acb,
 
 static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
-    CURLState *s = ((CURLState*)opaque);
+    CURLState *s = (CURLState *)opaque;
+    CURLDataCache *c = s->cache;
     size_t realsize = size * nmemb;
-    int i;
-
-    DPRINTF("CURL: Just reading %zd bytes\n", realsize);
+    CURLAIOCB *acb;
 
-    if (!s || !s->orig_buf)
+    if (!c || !c->data) {
         goto read_end;
+    }
+    if (c->write_pos >= c->data_len) {
+        goto read_end;
+    }
+    memcpy(c->data + c->write_pos, ptr,
+           MIN(realsize, c->data_len - c->write_pos));
+    c->write_pos += realsize;
+    if (c->write_pos >= c->data_len) {
+        c->write_pos = c->data_len;
+    }
 
-    memcpy(s->orig_buf + s->buf_off, ptr, realsize);
-    s->buf_off += realsize;
-
-    for(i=0; i<CURL_NUM_ACB; i++) {
-        CURLAIOCB *acb = s->acb[i];
-
-        if (!acb)
-            continue;
-
-        if ((s->buf_off >= acb->end)) {
-            qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start,
-                                acb->end - acb->start);
-            acb->common.cb(acb->common.opaque, 0);
-            qemu_aio_release(acb);
-            s->acb[i] = NULL;
+    acb = QLIST_FIRST(&s->s->acbs);
+    while (acb) {
+        size_t aio_base = acb->sector_num * SECTOR_SIZE;
+        size_t aio_len = acb->nb_sectors * SECTOR_SIZE;
+        CURLAIOCB *next = QLIST_NEXT(acb, next);
+        if (aio_base >= c->base_pos &&
+            aio_base + aio_len <= c->base_pos + c->write_pos) {
+            QLIST_REMOVE(acb, next);
+            curl_complete_io(s->s, acb, c);
         }
+        acb = next;
     }
 
 read_end:
@@ -275,10 +273,12 @@ static void curl_fd_handler(void *arg)
         CURLMsg *msg;
         msg = curl_multi_info_read(s->multi, &msgs_in_queue);
 
-        if (!msg)
+        if (!msg) {
             break;
-        if (msg->msg == CURLMSG_NONE)
+        }
+        if (msg->msg == CURLMSG_NONE) {
             break;
+        }
 
         switch (msg->msg) {
             case CURLMSG_DONE:
@@ -288,19 +288,17 @@ static void curl_fd_handler(void *arg)
                                   CURLINFO_PRIVATE,
                                   (char **)&state);
 
-                /* ACBs for successful messages get completed in curl_read_cb */
+                /* ACBs for successful messages get completed in curl_read_cb,
+                 * fail existing acbs for now */
                 if (msg->data.result != CURLE_OK) {
-                    int i;
-                    for (i = 0; i < CURL_NUM_ACB; i++) {
-                        CURLAIOCB *acb = state->acb[i];
-
-                        if (acb == NULL) {
-                            continue;
-                        }
-
+                    CURLAIOCB *acb = QLIST_FIRST(&s->acbs);
+                    while (acb) {
+                        CURLAIOCB *next = QLIST_NEXT(acb, next);
+                        DPRINTF("EIO, %s\n", state->errmsg);
                         acb->common.cb(acb->common.opaque, -EIO);
+                        QLIST_REMOVE(acb, next);
                         qemu_aio_release(acb);
-                        state->acb[i] = NULL;
+                        acb = next;
                     }
                 }
 
@@ -317,13 +315,10 @@ static void curl_fd_handler(void *arg)
 static CURLState *curl_init_state(BDRVCURLState *s)
 {
     CURLState *state = NULL;
-    int i, j;
+    int i;
 
     do {
         for (i=0; i<CURL_NUM_STATES; i++) {
-            for (j=0; j<CURL_NUM_ACB; j++)
-                if (s->states[i].acb[j])
-                    continue;
             if (s->states[i].in_use)
                 continue;
 
@@ -380,6 +375,10 @@ static void curl_clean_state(CURLState *s)
     if (s->s->multi)
         curl_multi_remove_handle(s->s->multi, s->curl);
     s->in_use = 0;
+    if (s->cache) {
+        s->cache->use_count--;
+        assert(s->cache->use_count >= 0);
+    }
 }
 
 static void curl_parse_filename(const char *filename, QDict *options,
@@ -483,6 +482,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
 
     QLIST_INIT(&s->socks);
     QLIST_INIT(&s->cache);
+    QLIST_INIT(&s->acbs);
 
     DPRINTF("CURL: Opening %s\n", file);
     s->url = g_strdup(file);
@@ -551,14 +551,8 @@ out_noclean:
 static int curl_aio_flush(void *opaque)
 {
     BDRVCURLState *s = opaque;
-    int i, j;
-
-    for (i=0; i < CURL_NUM_STATES; i++) {
-        for(j=0; j < CURL_NUM_ACB; j++) {
-            if (s->states[i].acb[j]) {
-                return 1;
-            }
-        }
+    if (!QLIST_EMPTY(&s->acbs)) {
+        return 1;
     }
     return 0;
 }
@@ -580,6 +574,7 @@ static void curl_readv_bh_cb(void *p)
     CURLDataCache *cache = NULL;
     CURLAIOCB *acb = p;
     BDRVCURLState *s = acb->common.bs->opaque;
+    int running;
     size_t aio_base, aio_bytes;
 
     qemu_bh_delete(acb->bh);
@@ -588,8 +583,9 @@ static void curl_readv_bh_cb(void *p)
     aio_base = acb->sector_num * SECTOR_SIZE;
     aio_bytes = acb->nb_sectors * SECTOR_SIZE;
 
-    size_t start = acb->sector_num * SECTOR_SIZE;
-    size_t end;
+    if (aio_base + aio_bytes > s->len) {
+        goto err_release;
+    }
 
     cache = curl_find_cache(s, aio_base, aio_bytes);
     if (cache) {
@@ -600,29 +596,41 @@ static void curl_readv_bh_cb(void *p)
     // No cache found, so let's start a new request
     state = curl_init_state(s);
     if (!state) {
-        acb->common.cb(acb->common.opaque, -EIO);
-        qemu_aio_release(acb);
-        return;
+        goto err_release;
     }
 
-    acb->start = 0;
-    acb->end = (acb->nb_sectors * SECTOR_SIZE);
-
-    state->buf_off = 0;
-    if (state->orig_buf)
-        g_free(state->orig_buf);
-    state->buf_start = start;
-    state->buf_len = acb->end + s->readahead_size;
-    end = MIN(start + state->buf_len, s->len) - 1;
-    state->orig_buf = g_malloc(state->buf_len);
-    state->acb[0] = acb;
-
-    snprintf(state->range, sizeof(state->range) - 1, "%zd-%zd", start, end);
-    DPRINTF("CURL (AIO): Reading %d at %zd (%s)\n",
-            (acb->nb_sectors * SECTOR_SIZE), start, state->range);
-    curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
+    cache = g_malloc0(sizeof(CURLDataCache));
+    cache->base_pos = acb->sector_num * SECTOR_SIZE;
+    cache->data_len = aio_bytes + s->readahead_size;
+    cache->write_pos = 0;
+    cache->data = g_malloc(cache->data_len);
 
+    QLIST_INSERT_HEAD(&s->acbs, acb, next);
+    snprintf(state->range, sizeof(state->range) - 1, "%zd-%zd", cache->base_pos,
+             cache->base_pos + cache->data_len);
+    DPRINTF("Reading range: %s\n", state->range);
+    curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
+    QLIST_INSERT_HEAD(&s->cache, cache, next);
+    state->cache = cache;
+    cache->use_count++;
     curl_multi_add_handle(s->multi, state->curl);
+    /* kick off curl to start the action */
+    curl_multi_socket_action(s->multi, 0, CURL_SOCKET_TIMEOUT, &running);
+    return;
+
+err_release:
+    if (cache) {
+        if (cache->data) {
+            g_free(cache->data);
+            cache->data = NULL;
+        }
+        g_free(cache);
+        cache = NULL;
+    }
+    acb->common.cb(acb->common.opaque, -EIO);
+    qemu_aio_release(acb);
+    return;
+
 
 }
 
@@ -669,14 +677,18 @@ static void curl_close(BlockDriverState *bs)
             curl_easy_cleanup(s->states[i].curl);
             s->states[i].curl = NULL;
         }
-        if (s->states[i].orig_buf) {
-            g_free(s->states[i].orig_buf);
-            s->states[i].orig_buf = NULL;
-        }
     }
     if (s->multi)
         curl_multi_cleanup(s->multi);
 
+    while (!QLIST_EMPTY(&s->acbs)) {
+        CURLAIOCB *acb = QLIST_FIRST(&s->acbs);
+        acb->common.cb(acb->common.opaque, -EIO);
+        QLIST_REMOVE(acb, next);
+        qemu_aio_release(acb);
+        acb = NULL;
+    }
+
     while (!QLIST_EMPTY(&s->cache)) {
         CURLDataCache *cache = QLIST_FIRST(&s->cache);
         assert(cache->use_count == 0);
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 08/10] curl: use list to store CURLState

[Fam Zheng]

Make it consistent to other structures to use QLIST to store CURLState.
It also simplifies initialization and releasing of data.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 96 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 51 insertions(+), 45 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 4c4752b..3ea2552 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -38,7 +38,6 @@
                    CURLPROTO_FTP | CURLPROTO_FTPS | \
                    CURLPROTO_TFTP)
 
-#define CURL_NUM_STATES 8
 #define SECTOR_SIZE     512
 #define READ_AHEAD_SIZE (256 * 1024)
 
@@ -64,14 +63,13 @@ typedef struct CURLDataCache {
     QLIST_ENTRY(CURLDataCache) next;
 } CURLDataCache;
 
-typedef struct CURLState
-{
+typedef struct CURLState {
     struct BDRVCURLState *s;
     CURL *curl;
     char range[128];
     char errmsg[CURL_ERROR_SIZE];
     CURLDataCache *cache;
-    char in_use;
+    QLIST_ENTRY(CURLState) next;
 } CURLState;
 
 typedef struct CURLSockInfo {
@@ -84,7 +82,7 @@ typedef struct CURLSockInfo {
 typedef struct BDRVCURLState {
     CURLM *multi;
     size_t len;
-    CURLState states[CURL_NUM_STATES];
+    QLIST_HEAD(, CURLState) curl_states;
     QLIST_HEAD(, CURLAIOCB) acbs;
     QLIST_HEAD(, CURLSockInfo) socks;
     char *url;
@@ -303,6 +301,9 @@ static void curl_fd_handler(void *arg)
                 }
 
                 curl_clean_state(state);
+                QLIST_REMOVE(state, next);
+                g_free(state);
+                state = NULL;
                 break;
             }
             default:
@@ -314,29 +315,17 @@ static void curl_fd_handler(void *arg)
 
 static CURLState *curl_init_state(BDRVCURLState *s)
 {
-    CURLState *state = NULL;
-    int i;
-
-    do {
-        for (i=0; i<CURL_NUM_STATES; i++) {
-            if (s->states[i].in_use)
-                continue;
-
-            state = &s->states[i];
-            state->in_use = 1;
-            break;
-        }
-        if (!state) {
-            g_usleep(100);
-        }
-    } while(!state);
-
-    if (state->curl)
-        goto has_curl;
+    CURLState *state;
 
+    state = g_malloc0(sizeof(CURLState));
+    state->s = s;
     state->curl = curl_easy_init();
-    if (!state->curl)
-        return NULL;
+    if (!state->curl) {
+        DPRINTF("CURL: curl_easy_init failed\n");
+        g_free(state);
+        state = NULL;
+        goto out;
+    }
     curl_easy_setopt(state->curl, CURLOPT_URL, s->url);
     curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5);
     curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb);
@@ -362,19 +351,19 @@ static CURLState *curl_init_state(BDRVCURLState *s)
 #ifdef DEBUG_VERBOSE
     curl_easy_setopt(state->curl, CURLOPT_VERBOSE, 1);
 #endif
-
-has_curl:
-
-    state->s = s;
-
+out:
     return state;
 }
 
 static void curl_clean_state(CURLState *s)
 {
-    if (s->s->multi)
-        curl_multi_remove_handle(s->s->multi, s->curl);
-    s->in_use = 0;
+    if (s->curl) {
+        if (s->s->multi) {
+            curl_multi_remove_handle(s->s->multi, s->curl);
+        }
+        curl_easy_cleanup(s->curl);
+        s->curl = NULL;
+    }
     if (s->cache) {
         s->cache->use_count--;
         assert(s->cache->use_count >= 0);
@@ -483,6 +472,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     QLIST_INIT(&s->socks);
     QLIST_INIT(&s->cache);
     QLIST_INIT(&s->acbs);
+    QLIST_INIT(&s->curl_states);
 
     DPRINTF("CURL: Opening %s\n", file);
     s->url = g_strdup(file);
@@ -520,7 +510,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
 
     curl_clean_state(state);
     curl_easy_cleanup(state->curl);
-    state->curl = NULL;
+    g_free(state);
+    state = NULL;
 
     // Now we know the file exists and its size, so let's
     // initialize the multi interface!
@@ -610,6 +601,7 @@ static void curl_readv_bh_cb(void *p)
              cache->base_pos + cache->data_len);
     DPRINTF("Reading range: %s\n", state->range);
     curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
+    QLIST_INSERT_HEAD(&s->curl_states, state, next);
     QLIST_INSERT_HEAD(&s->cache, cache, next);
     state->cache = cache;
     cache->use_count++;
@@ -631,7 +623,6 @@ err_release:
     qemu_aio_release(acb);
     return;
 
-
 }
 
 static BlockDriverAIOCB *curl_aio_readv(BlockDriverState *bs,
@@ -660,9 +651,13 @@ static BlockDriverAIOCB *curl_aio_readv(BlockDriverState *bs,
 static void curl_close(BlockDriverState *bs)
 {
     BDRVCURLState *s = bs->opaque;
-    int i;
 
     DPRINTF("CURL: Close\n");
+    if (s->timer) {
+        qemu_del_timer(s->timer);
+        qemu_free_timer(s->timer);
+        s->timer = NULL;
+    }
 
     if (s->timer) {
         qemu_del_timer(s->timer);
@@ -670,16 +665,26 @@ static void curl_close(BlockDriverState *bs)
         s->timer = NULL;
     }
 
-    for (i=0; i<CURL_NUM_STATES; i++) {
-        if (s->states[i].in_use)
-            curl_clean_state(&s->states[i]);
-        if (s->states[i].curl) {
-            curl_easy_cleanup(s->states[i].curl);
-            s->states[i].curl = NULL;
-        }
+    while (!QLIST_EMPTY(&s->curl_states)) {
+        CURLState *state = QLIST_FIRST(&s->curl_states);
+        /* Remove and clean curl easy handles */
+        curl_clean_state(state);
+        QLIST_REMOVE(state, next);
+        g_free(state);
+        state = NULL;
     }
-    if (s->multi)
+
+    if (s->multi) {
         curl_multi_cleanup(s->multi);
+    }
+
+    while (!QLIST_EMPTY(&s->acbs)) {
+        CURLAIOCB *acb = QLIST_FIRST(&s->acbs);
+        acb->common.cb(acb->common.opaque, -EIO);
+        QLIST_REMOVE(acb, next);
+        qemu_aio_release(acb);
+        acb = NULL;
+    }
 
     while (!QLIST_EMPTY(&s->acbs)) {
         CURLAIOCB *acb = QLIST_FIRST(&s->acbs);
@@ -709,6 +714,7 @@ static void curl_close(BlockDriverState *bs)
     }
 
     g_free(s->url);
+    s->url = NULL;
 }
 
 static int64_t curl_getlength(BlockDriverState *bs)
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 09/10] curl: add cache quota.

[Fam Zheng]

Introduce a cache quota: BDRVCURLState.cache_quota.
When adding new CURLDataCache to BDRVCURLState, if number of existing
CURLDataCache is larger than CURL_CACHE_QUOTA, try to release some first
to limit the in memory cache size.

A least used entry is selected for releasing.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 3ea2552..5adbc84 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -40,6 +40,7 @@
 
 #define SECTOR_SIZE     512
 #define READ_AHEAD_SIZE (256 * 1024)
+#define CURL_CACHE_QUOTA 10
 
 struct BDRVCURLState;
 
@@ -90,6 +91,8 @@ typedef struct BDRVCURLState {
     QEMUTimer *timer;
     /* List of data cache ordered by access, freed from tail */
     QLIST_HEAD(, CURLDataCache) cache;
+    /* Threshold to release unused cache when cache list is longer than it */
+    int cache_quota;
     /* Whether http server accept range in header */
     bool accept_range;
 } BDRVCURLState;
@@ -526,6 +529,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
     curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
 
     qemu_opts_del(opts);
+    s->cache_quota = CURL_CACHE_QUOTA;
+
     return 0;
 
 out:
@@ -595,6 +600,27 @@ static void curl_readv_bh_cb(void *p)
     cache->data_len = aio_bytes + s->readahead_size;
     cache->write_pos = 0;
     cache->data = g_malloc(cache->data_len);
+    /* Try to release some cache */
+    while (0 && s->cache_quota <= 0) {
+        CURLDataCache *p;
+        CURLDataCache *q = NULL;
+        assert(!QLIST_EMPTY(&s->cache));
+        for (p = QLIST_FIRST(&s->cache);
+             p; p = QLIST_NEXT(p, next)) {
+            if (p->use_count == 0) {
+                q = p;
+            }
+        }
+        if (!q) {
+            break;
+        }
+        QLIST_REMOVE(q, next);
+        g_free(q->data);
+        q->data = NULL;
+        g_free(q);
+        q = NULL;
+        s->cache_quota++;
+    }
 
     QLIST_INSERT_HEAD(&s->acbs, acb, next);
     snprintf(state->range, sizeof(state->range) - 1, "%zd-%zd", cache->base_pos,
@@ -605,6 +631,7 @@ static void curl_readv_bh_cb(void *p)
     QLIST_INSERT_HEAD(&s->cache, cache, next);
     state->cache = cache;
     cache->use_count++;
+    s->cache_quota--;
     curl_multi_add_handle(s->multi, state->curl);
     /* kick off curl to start the action */
     curl_multi_socket_action(s->multi, 0, CURL_SOCKET_TIMEOUT, &running);
-- 
1.8.2.3

[Qemu-devel] [PATCH v4 10/10] curl: introduce ssl_no_cert runtime option.

[Fam Zheng]

Added an option to let curl disable ssl certificate check.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 5adbc84..b6cc5a0 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -95,6 +95,8 @@ typedef struct BDRVCURLState {
     int cache_quota;
     /* Whether http server accept range in header */
     bool accept_range;
+    /* Whether certificated ssl only */
+    bool ssl_no_cert;
 } BDRVCURLState;
 
 static void curl_clean_state(CURLState *s);
@@ -339,6 +341,8 @@ static CURLState *curl_init_state(BDRVCURLState *s)
     curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1);
     curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg);
     curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1);
+    curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER,
+                     s->ssl_no_cert ? 0 : 1);
 
     /* Restrict supported protocols to avoid security issues in the more
      * obscure protocols.  For example, do not allow POP3/SMTP/IMAP see
@@ -429,7 +433,12 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_SIZE,
             .help = "Readahead size",
         },
-        { /* end of list */ }
+        {
+            .name = "ssl_no_cert",
+            .type = QEMU_OPT_BOOL,
+            .help = "SSL certificate check",
+        },
+        { /* End of list */ }
     },
 };
 
@@ -467,6 +476,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
         goto out_noclean;
     }
 
+    s->ssl_no_cert = qemu_opt_get_bool(opts, "ssl_no_cert", true);
     if (!inited) {
         curl_global_init(CURL_GLOBAL_ALL);
         inited = 1;
-- 
1.8.2.3

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Fam Zheng]

Rich,

Please try this version with libguestfs. Thanks.

(missing CC due to improper git send-email config)


On Wed, 05/22 11:16, Fam Zheng wrote:
> CURL library API has changed, the current curl driver is not working.
> This patch rewrites the use of API as well as the structure of internal
> states. 
> 
> BDRVCURLState holds the pointer to curl multi interface (man 3
> libcurl-multi), and 4 lists for internal states:
>  - CURLState holds state for libcurl connection (man 3 libcurl-easy)
>  - CURLSockInfo holds information for libcurl socket interface (man 3
>    curl_multi_socket_action).
>  - CURLDataCache holds the user data read from libcurl, it is in a list
>    ordered by access, the used cache is moved to list head on access, so
>    the tail element is freed first. BDRVCURLState.cache_quota is the
>    threshold to start freeing cache.
>  - CURLAIOCB holds ongoing aio information.
> 
> Changes from v3:
>   01, 06, 07: Add QLIST_INIT in qemu_open to initialize each list.
>   07: Move clean up for s->acbs from later patch to here. Use qemu_aio_relase instead of g_free on acb.
>       Fix use-after-free bug. [Rich]
> 
> Changes from v2:
>   00: Removed previous 09, since changes are included in previous
>       commits.
>       previous 09: curl: release everything on curl close.
>   01: Add commit message for introducing CURLSockInfo. Remove
>       unnecessary pointer type cast.
>   02: Use sizeof instead of macro for s->range len.
>   04: Use pstrcpy and strncasecmp.
>   06: Add commit message for why is there a cache in curl.c.
> 
> Changes from v1:
>    Split to multiple patches. The final status should be identical to v1.
> 
> Fam Zheng (10):
>   curl: introduce CURLSockInfo to BDRVCURLState.
>   curl: change magic number to sizeof
>   curl: change curl_multi_do to curl_fd_handler
>   curl: fix curl_open
>   curl: add timer to BDRVCURLState
>   curl: introduce CURLDataCache
>   curl: make use of CURLDataCache.
>   curl: use list to store CURLState
>   curl: add cache quota.
>   curl: introduce ssl_no_cert runtime option.
> 
>  block/curl.c | 574 +++++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 361 insertions(+), 213 deletions(-)
> 
> -- 
> 1.8.2.3
> 
> 

-- 
Fam

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Richard W.M. Jones]

On Wed, May 22, 2013 at 11:16:40AM +0800, Fam Zheng wrote:
> Changes from v3:
>   01, 06, 07: Add QLIST_INIT in qemu_open to initialize each list.
>   07: Move clean up for s->acbs from later patch to here. Use qemu_aio_relase instead of g_free on acb.
>       Fix use-after-free bug. [Rich]

This version definitely works better.  I'm able to open the guest and
perform some simple guestfish commands.

  - - -

However I'm still able to hit a crash in qemu, *but* it appears this
crash has nothing to do with your code.  The same crash happens
whether or not I apply your patch, and occurs in common block code.

Here are the details of that other crash (qemu from git w/o your patch,
curl from git):

$ LD_LIBRARY_PATH=~/d/curl/lib/.libs LIBGUESTFS_QEMU=~/d/qemu/qemu.wrapper LIBGUESTFS_BACKEND=direct ./run ./fish/guestfish -a http://192.168.0.249/scratch/winxp.img -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: Microsoft Windows XP
/dev/sda1 mounted on /

><fs> find /
libguestfs: error: appliance closed the connection unexpectedly.
This usually means the libguestfs appliance crashed.
See http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
for information about how to debug libguestfs and report bugs.
libguestfs: error: /home/rjones/d/qemu/qemu.wrapper killed by signal 11 (Segmentation fault).
To see full error messages you may need to enable debugging.
See http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
libguestfs: error: receive_file_data: parse error in reply callback
libguestfs: error: /home/rjones/d/libguestfs/tmp/libguestfsN7yyt4/find1: error in chunked encoding

[...]

Program terminated with signal 11, Segmentation fault.
#0  0x0000000000000000 in ?? ()
Missing separate debuginfos, use: debuginfo-install SDL-1.2.15-3.fc18.x86_64 bluez-libs-4.101-6.fc18.x86_64 brlapi-0.5.6-12.fc18.x86_64 celt051-0.5.1.3-5.fc18.x86_64 ceph-devel-0.56.3-1.fc18.x86_64 ceph-libs-0.56.3-1.fc18.x86_64 cryptopp-5.6.2-2.fc18.x86_64 cyrus-sasl-lib-2.1.25-2.fc18.x86_64 glusterfs-3.4.0-0.4.beta1.fc20.x86_64 glusterfs-api-3.4.0-0.4.beta1.fc20.x86_64 leveldb-1.7.0-4.fc18.x86_64 libfdt-1.3.0-5.fc18.x86_64 libseccomp-1.0.1-0.fc18.x86_64 libselinux-2.1.12-7.3.fc18.x86_64 libusbx-1.0.14-1.fc18.x86_64 nss-mdns-0.10-11.fc18.x86_64 snappy-1.0.5-2.fc18.x86_64 spice-server-0.12.2-3.fc18.x86_64 usbredir-0.6-1.fc18.x86_64 xen-libs-4.2.2-3.fc18.x86_64
(gdb) t a a bt

Thread 6 (Thread 0x7f659cb28700 (LWP 21380)):
#0  sem_timedwait ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x00007f65ad2c6c0b in qemu_sem_timedwait (sem=sem@entry=0x7f65ae0ffe08, 
    ms=ms@entry=10000) at util/qemu-thread-posix.c:237
#2  0x00007f65ad1843ce in worker_thread (opaque=0x7f65ae0ffd70)
    at thread-pool.c:96
#3  0x00007f65ab1cdd15 in start_thread (arg=0x7f659cb28700)
    at pthread_create.c:308
#4  0x00007f65a5b9548d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114

Thread 5 (Thread 0x7f659d329700 (LWP 21379)):
#0  sem_timedwait ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x00007f65ad2c6c0b in qemu_sem_timedwait (sem=sem@entry=0x7f65ae0ffe08, 
    ms=ms@entry=10000) at util/qemu-thread-posix.c:237
#2  0x00007f65ad1843ce in worker_thread (opaque=0x7f65ae0ffd70)
    at thread-pool.c:96
#3  0x00007f65ab1cdd15 in start_thread (arg=0x7f659d329700)
    at pthread_create.c:308
#4  0x00007f65a5b9548d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114

Thread 4 (Thread 0x7f659db2a700 (LWP 21378)):
#0  sem_timedwait ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x00007f65ad2c6c0b in qemu_sem_timedwait (sem=sem@entry=0x7f65ae0ffe08, 
    ms=ms@entry=10000) at util/qemu-thread-posix.c:237
#2  0x00007f65ad1843ce in worker_thread (opaque=0x7f65ae0ffd70)
    at thread-pool.c:96
#3  0x00007f65ab1cdd15 in start_thread (arg=0x7f659db2a700)
    at pthread_create.c:308
#4  0x00007f65a5b9548d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114

Thread 3 (Thread 0x7f659eb2c700 (LWP 21376)):
#0  sem_timedwait ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/sem_timedwait.S:101
#1  0x00007f65ad2c6c0b in qemu_sem_timedwait (sem=sem@entry=0x7f65ae0ffe08, 
    ms=ms@entry=10000) at util/qemu-thread-posix.c:237
#2  0x00007f65ad1843ce in worker_thread (opaque=0x7f65ae0ffd70)
    at thread-pool.c:96
#3  0x00007f65ab1cdd15 in start_thread (arg=0x7f659eb2c700)
    at pthread_create.c:308
#4  0x00007f65a5b9548d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114

Thread 2 (Thread 0x7f659fab7700 (LWP 21373)):
#0  0x00007f65a5b8e0e7 in ioctl () at ../sysdeps/unix/syscall-template.S:81
#1  0x00007f65ad20ffc9 in kvm_vcpu_ioctl (cpu=cpu@entry=0x7f65ae47ce90, 
    type=type@entry=44672) at /home/rjones/d/qemu/kvm-all.c:1729
#2  0x00007f65ad210155 in kvm_cpu_exec (env=env@entry=0x7f65ae47cfa0)
    at /home/rjones/d/qemu/kvm-all.c:1614
#3  0x00007f65ad1b1975 in qemu_kvm_cpu_thread_fn (arg=0x7f65ae47cfa0)
    at /home/rjones/d/qemu/cpus.c:759
#4  0x00007f65ab1cdd15 in start_thread (arg=0x7f659fab7700)
    at pthread_create.c:308
#5  0x00007f65a5b9548d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:114

Thread 1 (Thread 0x7f65acf0ea00 (LWP 21370)):
#0  0x0000000000000000 in ?? ()
#1  0x00007f65ad005b3e in bdrv_co_io_em (is_write=true, iov=0x7f65b0b55228, 
    nb_sectors=8, sector_num=2104919, bs=0x7f65ae0d26f0) at block.c:4283
#2  bdrv_co_writev_em (bs=0x7f65ae0d26f0, sector_num=2104919, nb_sectors=8, 
    iov=0x7f65b0b55228) at block.c:4310
#3  0x00007f65ad00c125 in bdrv_co_do_writev (bs=0x7f65ae0d26f0, sector_num=
    2104919, nb_sectors=8, qiov=0x7f65b0b55228, flags=(unknown: 0))
    at block.c:2625
#4  0x00007f65ad00c125 in bdrv_co_do_writev (bs=0x7f65ae0cfcd0, sector_num=
    2104919, nb_sectors=8, qiov=0x7f65b0b55228, flags=flags@entry=(unknown: 0))
    at block.c:2625
#5  0x00007f65ad00c288 in bdrv_co_do_rw (opaque=0x7f65afbed140) at block.c:4139
#6  0x00007f65ad0436ea in coroutine_trampoline (i0=<optimized out>, 
    i1=<optimized out>) at coroutine-ucontext.c:118
#7  0x00007f65a5ae96c0 in ?? () from /lib64/libc.so.6
#8  0x00007fff88fd1ab0 in ?? ()
#9  0xe0e0e0e0e0e0e0e0 in ?? ()
#10 0x0000000000000000 in ?? ()

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Paolo Bonzini]

Il 22/05/2013 11:44, Richard W.M. Jones ha scritto:
> On Wed, May 22, 2013 at 11:16:40AM +0800, Fam Zheng wrote:
>> Changes from v3:
>>   01, 06, 07: Add QLIST_INIT in qemu_open to initialize each list.
>>   07: Move clean up for s->acbs from later patch to here. Use qemu_aio_relase instead of g_free on acb.
>>       Fix use-after-free bug. [Rich]
> 
> This version definitely works better.  I'm able to open the guest and
> perform some simple guestfish commands.
> 
>   - - -
> 
> However I'm still able to hit a crash in qemu, *but* it appears this
> crash has nothing to do with your code.  The same crash happens
> whether or not I apply your patch, and occurs in common block code.
> 
> Here are the details of that other crash (qemu from git w/o your patch,
> curl from git):
> 
> $ LD_LIBRARY_PATH=~/d/curl/lib/.libs LIBGUESTFS_QEMU=~/d/qemu/qemu.wrapper LIBGUESTFS_BACKEND=direct ./run ./fish/guestfish -a http://192.168.0.249/scratch/winxp.img -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: Microsoft Windows XP
> /dev/sda1 mounted on /
> 
>> <fs> find /
> libguestfs: error: appliance closed the connection unexpectedly.
> This usually means the libguestfs appliance crashed.
> See http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
> for information about how to debug libguestfs and report bugs.
> libguestfs: error: /home/rjones/d/qemu/qemu.wrapper killed by signal 11 (Segmentation fault).
> To see full error messages you may need to enable debugging.
> See http://libguestfs.org/guestfs-faq.1.html#debugging-libguestfs
> libguestfs: error: receive_file_data: parse error in reply callback
> libguestfs: error: /home/rjones/d/libguestfs/tmp/libguestfsN7yyt4/find1: error in chunked encoding
> 
> [...]
> 
> Program terminated with signal 11, Segmentation fault.
> Thread 1 (Thread 0x7f65acf0ea00 (LWP 21370)):
> #0  0x0000000000000000 in ?? ()
> #1  0x00007f65ad005b3e in bdrv_co_io_em (is_write=true, iov=0x7f65b0b55228, 
>     nb_sectors=8, sector_num=2104919, bs=0x7f65ae0d26f0) at block.c:4283

Something is trying to write, but there's no write operation defined for
CURL.

I guess curl (and other backends too) should reject being opened for
write.  Alternatively, block.c could do that for them.

Paolo

> #2  bdrv_co_writev_em (bs=0x7f65ae0d26f0, sector_num=2104919, nb_sectors=8, 
>     iov=0x7f65b0b55228) at block.c:4310
> #3  0x00007f65ad00c125 in bdrv_co_do_writev (bs=0x7f65ae0d26f0, sector_num=
>     2104919, nb_sectors=8, qiov=0x7f65b0b55228, flags=(unknown: 0))
>     at block.c:2625
> #4  0x00007f65ad00c125 in bdrv_co_do_writev (bs=0x7f65ae0cfcd0, sector_num=
>     2104919, nb_sectors=8, qiov=0x7f65b0b55228, flags=flags@entry=(unknown: 0))
>     at block.c:2625
> #5  0x00007f65ad00c288 in bdrv_co_do_rw (opaque=0x7f65afbed140) at block.c:4139
> #6  0x00007f65ad0436ea in coroutine_trampoline (i0=<optimized out>, 
>     i1=<optimized out>) at coroutine-ucontext.c:118
> #7  0x00007f65a5ae96c0 in ?? () from /lib64/libc.so.6
> #8  0x00007fff88fd1ab0 in ?? ()
> #9  0xe0e0e0e0e0e0e0e0 in ?? ()
> #10 0x0000000000000000 in ?? ()
> 
> Rich.
> 

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Richard W.M. Jones]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Wed, May 22, 2013 at 01:04:51PM +0200, Paolo Bonzini wrote:
> Something is trying to write, but there's no write operation defined for
> CURL.
> 
> I guess curl (and other backends too) should reject being opened for
> write.  Alternatively, block.c could do that for them.

Yes, I'd just got to that conclusion as well :-)

The attached patch fixes the crash for me.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

[-- Attachment #2: 0001-block-curl.c-Refuse-to-open-the-handle-for-writes.patch --]
[-- Type: text/plain, Size: 771 bytes --]

From 0d42755f009c142b85fb07a06a41cee6c7ea68ba Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Wed, 22 May 2013 12:11:45 +0100
Subject: [PATCH] block/curl.c: Refuse to open the handle for writes.

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

diff --git a/block/curl.c b/block/curl.c
index b8935fd..f1e302b 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -406,6 +406,10 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags)
 
     static int inited = 0;
 
+    if (flags & BDRV_O_RDWR) {
+        return -ENOTSUP;
+    }
+
     opts = qemu_opts_create_nofail(&runtime_opts);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
-- 
1.8.2.1

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Stefan Hajnoczi]

On Wed, May 22, 2013 at 12:12:24PM +0100, Richard W.M. Jones wrote:
> On Wed, May 22, 2013 at 01:04:51PM +0200, Paolo Bonzini wrote:
> > Something is trying to write, but there's no write operation defined for
> > CURL.
> > 
> > I guess curl (and other backends too) should reject being opened for
> > write.  Alternatively, block.c could do that for them.
> 
> Yes, I'd just got to that conclusion as well :-)
> 
> The attached patch fixes the crash for me.

Please post a top-level thread so that your patch gets picked up by
scripts and noticed by humans too :).

Alternatively Fam can include it in the next revision of this series.

Stefan

Re: [Qemu-devel] [PATCH v4 00/10] curl: fix curl read

[Fam Zheng]

On Wed, 05/22 16:39, Stefan Hajnoczi wrote:
> On Wed, May 22, 2013 at 12:12:24PM +0100, Richard W.M. Jones wrote:
> > On Wed, May 22, 2013 at 01:04:51PM +0200, Paolo Bonzini wrote:
> > > Something is trying to write, but there's no write operation defined for
> > > CURL.
> > > 
> > > I guess curl (and other backends too) should reject being opened for
> > > write.  Alternatively, block.c could do that for them.
> > 
> > Yes, I'd just got to that conclusion as well :-)
> > 
> > The attached patch fixes the crash for me.
> 
> Please post a top-level thread so that your patch gets picked up by
> scripts and noticed by humans too :).
> 
> Alternatively Fam can include it in the next revision of this series.
> 
> Stefan
> 

No problem, I'll post a new version to include it.

-- 
Fam