From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHZMr-0003GN-Ir for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:17:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHZMi-0005eh-Gq for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:17:33 -0400 Received: from e24smtp02.br.ibm.com ([32.104.18.86]:57561) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHZMi-0005e9-5d for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:17:24 -0400 Received: from /spool/local by e24smtp02.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 13 Aug 2014 11:17:22 -0300 Received: from d24relay02.br.ibm.com (d24relay02.br.ibm.com [9.13.184.26]) by d24dlp01.br.ibm.com (Postfix) with ESMTP id 753073520068 for ; Wed, 13 Aug 2014 10:17:12 -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 s7DEGLGh12124446 for ; Wed, 13 Aug 2014 11:16:21 -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 s7DEHIrC031152 for ; Wed, 13 Aug 2014 11:17:18 -0300 Message-ID: <53EB736D.8020208@linux.vnet.ibm.com> Date: Wed, 13 Aug 2014 11:17:17 -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> <53EB6809.40809@linux.vnet.ibm.com> <87r40kwkiq.fsf@blackfin.pond.sub.org> In-Reply-To: <87r40kwkiq.fsf@blackfin.pond.sub.org> 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 11:07 AM, Markus Armbruster wrote: > Daniel H Barboza writes: > >> 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? > I can't see other block driver options with names of the form > DRVNAME-OPTNAME, so keeping things standardized would rather call for > just "timeout". > > Just "timeout" seems clear enough to me. But if you can think of > concrete contexts where it would be confusing, then I don't mind > "curl-timeout". > I think you have a point. Since I can't think of any concrete context where it would be confusing, I'll rename the option to "timeout" in v3. Thanks!