Patchwork [v2,01/11] curl: introduce CURLSockInfo to BDRVCURLState.

login
register
mail settings
Submitter Fam Zheng
Date May 14, 2013, 2:26 a.m.
Message ID <1368498390-20738-2-git-send-email-famz@redhat.com>
Download mbox | patch
Permalink /patch/243561/
State New
Headers show

Comments

Fam Zheng - May 14, 2013, 2:26 a.m.
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.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
Stefan Hajnoczi - May 14, 2013, 7:59 a.m.
On Tue, May 14, 2013 at 10:26:20AM +0800, Fam Zheng wrote:
> 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.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/curl.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index b8935fd..14f4552 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;

This patch doesn't make sense by itself.  CURLSockInfo is unused and
->action is never assigned to.  I guess this will eventually replace
BDRVCURLState->states[] but at this point in the patch series it is hard
to tell - it would be nice to explain this in the commit description.

>  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 = (BDRVCURLState *)s;

void * is automatically coerced to any pointer type.  No need for a type
cast.

>      DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, fd);
> +    CURLSockInfo *sock = (CURLSockInfo *)sp;

Same here.

> +    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);
> @@ -462,8 +479,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);
> -- 
> 1.8.1.4
> 
>

Patch

diff --git a/block/curl.c b/block/curl.c
index b8935fd..14f4552 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 = (BDRVCURLState *)s;
     DPRINTF("CURL (AIO): Sock action %d on fd %d\n", action, fd);
+    CURLSockInfo *sock = (CURLSockInfo *)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);
@@ -462,8 +479,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);