From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57233) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHYbp-000084-UO for qemu-devel@nongnu.org; Wed, 13 Aug 2014 09:29:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHYbg-0003X3-Qm for qemu-devel@nongnu.org; Wed, 13 Aug 2014 09:28:57 -0400 Received: from e24smtp03.br.ibm.com ([32.104.18.24]:46484) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHYbg-0003Wz-Fl for qemu-devel@nongnu.org; Wed, 13 Aug 2014 09:28:48 -0400 Received: from /spool/local by e24smtp03.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Aug 2014 10:28:46 -0300 Received: from d24relay01.br.ibm.com (d24relay01.br.ibm.com [9.8.31.16]) by d24dlp02.br.ibm.com (Postfix) with ESMTP id 95B841DC0027 for ; Wed, 13 Aug 2014 09:28:42 -0400 (EDT) Received: from d24av03.br.ibm.com (d24av03.br.ibm.com [9.8.31.95]) by d24relay01.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s7DDT2oV4710448 for ; Wed, 13 Aug 2014 10:29:02 -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 s7DDSgpF018575 for ; Wed, 13 Aug 2014 10:28:42 -0300 Message-ID: <53EB6809.40809@linux.vnet.ibm.com> Date: Wed, 13 Aug 2014 10:28:41 -0300 From: Daniel H Barboza MIME-Version: 1.0 References: <1407854125-25068-1-git-send-email-danielhb@linux.vnet.ibm.com> <871tsk698v.fsf@blackfin.pond.sub.org> <53EB5C75.3020909@linux.vnet.ibm.com> In-Reply-To: <53EB5C75.3020909@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , Qemu Devel , Stefan Hajnoczi On 08/13/2014 09:39 AM, Daniel H Barboza wrote: > > On 08/13/2014 06:15 AM, Markus Armbruster wrote: >> Daniel Henrique Barboza writes: >> >>> 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" >> To what could this timeout apply other than Curl? >> >> If nothing, then just "timeout", please. >> >> Else, "curl-timeout". > > I will investigate a little to check if there are any option called > "timeout" used elsewhere. If not, I see no issues rename this option > to "timeout". > > Thanks! I've found one option that contains the string "timeout": vl.c static QemuOptsList qemu_boot_opts = { .name = "boot-opts", .implied_opt_name = "order", (...) }, { .name = "reboot-timeout", .type = QEMU_OPT_STRING, }, { (...) I think that renaming "curltimeout" to just "timeout" can be a little misleading depending on the context. However, I believe that to keep the name of the options standardized we can rename "curltimeout" to "curl-timeout" as you've suggested. Do you agree? Thanks > >>> struct BDRVCURLState; >>> @@ -109,6 +111,7 @@ typedef struct BDRVCURLState { >>> char *url; >>> size_t readahead_size; >>> bool sslverify; >>> + int curltimeout; >>> bool accept_range; >>> AioContext *aio_context; >>> } BDRVCURLState; >> Likewise: either timeout, or curl_timeout. >> >> [...] >> > >