From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59464) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHaYO-0004e8-Oc for qemu-devel@nongnu.org; Wed, 13 Aug 2014 11:33:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHaYG-0007vX-I7 for qemu-devel@nongnu.org; Wed, 13 Aug 2014 11:33:32 -0400 Received: from e24smtp01.br.ibm.com ([32.104.18.85]:58699) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHaYG-0007oa-6K for qemu-devel@nongnu.org; Wed, 13 Aug 2014 11:33:24 -0400 Received: from /spool/local by e24smtp01.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Aug 2014 12:33:20 -0300 Received: from d24relay02.br.ibm.com (d24relay02.br.ibm.com [9.13.184.26]) by d24dlp02.br.ibm.com (Postfix) with ESMTP id 5820E1DC005C for ; Wed, 13 Aug 2014 11:33:17 -0400 (EDT) Received: from d24av03.br.ibm.com (d24av03.br.ibm.com [9.8.31.95]) by d24relay02.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s7DFWKDu4063532 for ; Wed, 13 Aug 2014 12:32:20 -0300 Received: from d24av03.br.ibm.com (localhost [127.0.0.1]) by d24av03.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s7DFXGeA006338 for ; Wed, 13 Aug 2014 12:33:17 -0300 Message-ID: <53EB8532.4070804@linux.vnet.ibm.com> Date: Wed, 13 Aug 2014 12:33:06 -0300 From: Daniel H Barboza MIME-Version: 1.0 References: <1407870777-594-1-git-send-email-danielhb@linux.vnet.ibm.com> <20140813143800.GC3091@irqsave.net> In-Reply-To: <20140813143800.GC3091@irqsave.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2] block.curl: adding 'curltimeout' option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Beno=EEt_Canet?= Cc: Qemu Devel On 08/13/2014 11:38 AM, Benoît Canet wrote: > The Tuesday 12 Aug 2014 à 16:12:57 (-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. >> >> Signed-off-by: Daniel Henrique Barboza >> --- >> 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..a9e43f1 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 "curltimeout" >> >> struct BDRVCURLState; >> >> @@ -109,6 +111,7 @@ typedef struct BDRVCURLState { >> char *url; >> size_t readahead_size; >> bool sslverify; >> + int curltimeout; > I know the sslverify does an exception to the coding style rule but why not > name the field curl_timeout like it would be in every other part of qemu code ? I'll change the name of the field to "timeout" in v3 as suggested by Markus Armbruster. This fixes the coding style issue you've pointed out. Thanks! > >> 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->curltimeout); >> 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->curltimeout = 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..0358f38 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 curltimeout >> +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 >> +curltimeout 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.curltimeout":10@}' /tmp/test.qcow2 >> >> qemu-system-x86_64 -drive file=/tmp/test.qcow2 >> @end example >> -- >> 1.8.3.1 >> >> > Apart from the coding style issue. > > Reviewed-by: Benoit Canet >