[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option |
Date: |
Wed, 13 Aug 2014 16:07:25 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Daniel H Barboza <address@hidden> 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 <address@hidden> 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 <address@hidden>
>>>> ---
>>>> 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".