[Qemu-devel] [PATCH v3] block.curl: timeout option

[Qemu-devel] [PATCH v3] block.curl: timeout option

[Daniel Henrique Barboza]

Changes in v3:
- changed option name from 'curltimeout' to 'timeout'

Changes in v2:
- remove double quote from the int value in qemu-options.hx

Daniel Henrique Barboza (1):
  block.curl: adding 'timeout' option

 block/curl.c    | 13 ++++++++++++-
 qemu-options.hx | 10 ++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3] block.curl: adding 'timeout' option

[Daniel Henrique Barboza]

The curl hardcoded timeout (5 seconds) sometimes is not long
enough depending on the remote server configuration and network
traffic. The user should be able to set how much long he is
willing to wait for the connection.

Adding a new option to set this timeout gives the user this
flexibility. The previous default timeout of 5 seconds will be
used if this option is not present.

Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
---
 block/curl.c    | 13 ++++++++++++-
 qemu-options.hx | 10 ++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 79ff2f1..6f45547 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -63,6 +63,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
 #define CURL_NUM_ACB    8
 #define SECTOR_SIZE     512
 #define READ_AHEAD_DEFAULT (256 * 1024)
+#define CURL_TIMEOUT_DEFAULT 5
 
 #define FIND_RET_NONE   0
 #define FIND_RET_OK     1
@@ -71,6 +72,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
 #define CURL_BLOCK_OPT_URL       "url"
 #define CURL_BLOCK_OPT_READAHEAD "readahead"
 #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
+#define CURL_BLOCK_OPT_TIMEOUT "timeout"
 
 struct BDRVCURLState;
 
@@ -109,6 +111,7 @@ typedef struct BDRVCURLState {
     char *url;
     size_t readahead_size;
     bool sslverify;
+    int timeout;
     bool accept_range;
     AioContext *aio_context;
 } BDRVCURLState;
@@ -382,7 +385,7 @@ static CURLState *curl_init_state(BDRVCURLState *s)
         curl_easy_setopt(state->curl, CURLOPT_URL, s->url);
         curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER,
                          (long) s->sslverify);
-        curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5);
+        curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, s->timeout);
         curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION,
                          (void *)curl_read_cb);
         curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state);
@@ -489,6 +492,11 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Verify SSL certificate"
         },
+        {
+            .name = CURL_BLOCK_OPT_TIMEOUT,
+            .type = QEMU_OPT_NUMBER,
+            .help = "Curl timeout"
+        },
         { /* end of list */ }
     },
 };
@@ -525,6 +533,9 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
         goto out_noclean;
     }
 
+    s->timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
+                                     CURL_TIMEOUT_DEFAULT);
+
     s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
 
     file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
diff --git a/qemu-options.hx b/qemu-options.hx
index 96516c1..e93a94f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2351,6 +2351,11 @@ multiple of 512 bytes. It defaults to 256k.
 @item sslverify
 Whether to verify the remote server's certificate when connecting over SSL. It
 can have the value 'on' or 'off'. It defaults to 'on'.
+
+@item timeout
+Set the timeout in seconds of the CURL connection. This timeout is the time
+that CURL waits for a response from the remote server to get the size of the
+image to be downloaded. If not set, the default timeout of 5 seconds is used.
 @end table
 
 Note that when passing options to qemu explicitly, @option{driver} is the value
@@ -2372,9 +2377,10 @@ qemu-system-x86_64 -drive file=/tmp/Fedora-x86_64-20-20131211.1-sda.qcow2,copy-o
 @end example
 
 Example: boot from an image stored on a VMware vSphere server with a self-signed
-certificate using a local overlay for writes and a readahead of 64k
+certificate using a local overlay for writes, a readahead of 64k and a timeout
+of 10 seconds.
 @example
-qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, "file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1",, "file.sslverify":"off",, "file.readahead":"64k"@}' /tmp/test.qcow2
+qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, "file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1",, "file.sslverify":"off",, "file.readahead":"64k",, "file.timeout":10@}' /tmp/test.qcow2
 
 qemu-system-x86_64 -drive file=/tmp/test.qcow2
 @end example
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3] block.curl: adding 'timeout' option

[Benoît Canet]

The Wednesday 13 Aug 2014 à 12:44:27 (-0300), Daniel Henrique Barboza wrote :
> The curl hardcoded timeout (5 seconds) sometimes is not long
> enough depending on the remote server configuration and network
> traffic. The user should be able to set how much long he is
> willing to wait for the connection.
> 
> Adding a new option to set this timeout gives the user this
> flexibility. The previous default timeout of 5 seconds will be
> used if this option is not present.
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> ---
>  block/curl.c    | 13 ++++++++++++-
>  qemu-options.hx | 10 ++++++++--
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index 79ff2f1..6f45547 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -63,6 +63,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
>  #define CURL_NUM_ACB    8
>  #define SECTOR_SIZE     512
>  #define READ_AHEAD_DEFAULT (256 * 1024)
> +#define CURL_TIMEOUT_DEFAULT 5
>  
>  #define FIND_RET_NONE   0
>  #define FIND_RET_OK     1
> @@ -71,6 +72,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
>  #define CURL_BLOCK_OPT_URL       "url"
>  #define CURL_BLOCK_OPT_READAHEAD "readahead"
>  #define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
> +#define CURL_BLOCK_OPT_TIMEOUT "timeout"
>  
>  struct BDRVCURLState;
>  
> @@ -109,6 +111,7 @@ typedef struct BDRVCURLState {
>      char *url;
>      size_t readahead_size;
>      bool sslverify;
> +    int timeout;
>      bool accept_range;
>      AioContext *aio_context;
>  } BDRVCURLState;
> @@ -382,7 +385,7 @@ static CURLState *curl_init_state(BDRVCURLState *s)
>          curl_easy_setopt(state->curl, CURLOPT_URL, s->url);
>          curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER,
>                           (long) s->sslverify);
> -        curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5);
> +        curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, s->timeout);
>          curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION,
>                           (void *)curl_read_cb);
>          curl_easy_setopt(state->curl, CURLOPT_WRITEDATA, (void *)state);
> @@ -489,6 +492,11 @@ static QemuOptsList runtime_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "Verify SSL certificate"
>          },
> +        {
> +            .name = CURL_BLOCK_OPT_TIMEOUT,
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "Curl timeout"
> +        },
>          { /* end of list */ }
>      },
>  };
> @@ -525,6 +533,9 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
>          goto out_noclean;
>      }
>  
> +    s->timeout = qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOUT,
> +                                     CURL_TIMEOUT_DEFAULT);
> +
>      s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
>  
>      file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 96516c1..e93a94f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2351,6 +2351,11 @@ multiple of 512 bytes. It defaults to 256k.
>  @item sslverify
>  Whether to verify the remote server's certificate when connecting over SSL. It
>  can have the value 'on' or 'off'. It defaults to 'on'.
> +
> +@item timeout
> +Set the timeout in seconds of the CURL connection. This timeout is the time
> +that CURL waits for a response from the remote server to get the size of the
> +image to be downloaded. If not set, the default timeout of 5 seconds is used.
>  @end table
>  
>  Note that when passing options to qemu explicitly, @option{driver} is the value
> @@ -2372,9 +2377,10 @@ qemu-system-x86_64 -drive file=/tmp/Fedora-x86_64-20-20131211.1-sda.qcow2,copy-o
>  @end example
>  
>  Example: boot from an image stored on a VMware vSphere server with a self-signed
> -certificate using a local overlay for writes and a readahead of 64k
> +certificate using a local overlay for writes, a readahead of 64k and a timeout
> +of 10 seconds.
>  @example
> -qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, "file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1",, "file.sslverify":"off",, "file.readahead":"64k"@}' /tmp/test.qcow2
> +qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, "file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1",, "file.sslverify":"off",, "file.readahead":"64k",, "file.timeout":10@}' /tmp/test.qcow2
>  
>  qemu-system-x86_64 -drive file=/tmp/test.qcow2
>  @end example
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit.canet@nodalink.com>

Re: [Qemu-devel] [PATCH v3] block.curl: adding 'timeout' option

[Richard W.M. Jones]

On Wed, Aug 13, 2014 at 12:44:27PM -0300, Daniel Henrique Barboza wrote:
> The curl hardcoded timeout (5 seconds) sometimes is not long
> enough depending on the remote server configuration and network
> traffic. The user should be able to set how much long he is
> willing to wait for the connection.
> 
> Adding a new option to set this timeout gives the user this
> flexibility. The previous default timeout of 5 seconds will be
> used if this option is not present.
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>

You can add:

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

Re: [Qemu-devel] [PATCH v3] block.curl: timeout option

[Stefan Hajnoczi]

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

On Wed, Aug 13, 2014 at 12:44:26PM -0300, Daniel Henrique Barboza wrote:
> Changes in v3:
> - changed option name from 'curltimeout' to 'timeout'
> 
> Changes in v2:
> - remove double quote from the int value in qemu-options.hx
> 
> Daniel Henrique Barboza (1):
>   block.curl: adding 'timeout' option
> 
>  block/curl.c    | 13 ++++++++++++-
>  qemu-options.hx | 10 ++++++++--
>  2 files changed, 20 insertions(+), 3 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]