* [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
@ 2014-08-12 14:35 Daniel Henrique Barboza
2014-08-12 15:36 ` Eric Blake
2014-08-13 9:15 ` Markus Armbruster
0 siblings, 2 replies; 8+ messages in thread
From: Daniel Henrique Barboza @ 2014-08-12 14:35 UTC (permalink / raw)
To: Qemu Devel; +Cc: Kevin Wolf, Daniel Henrique Barboza, Stefan Hajnoczi
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 <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..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;
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..36f5c3d 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-12 14:35 [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option Daniel Henrique Barboza
@ 2014-08-12 15:36 ` Eric Blake
2014-08-12 16:04 ` Daniel H Barboza
2014-08-13 9:15 ` Markus Armbruster
1 sibling, 1 reply; 8+ messages in thread
From: Eric Blake @ 2014-08-12 15:36 UTC (permalink / raw)
To: Daniel Henrique Barboza, Qemu Devel; +Cc: Kevin Wolf, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]
On 08/12/2014 08:35 AM, 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 <danielhb@linux.vnet.ibm.com>
> ---
> block/curl.c | 13 ++++++++++++-
> qemu-options.hx | 10 ++++++++--
> 2 files changed, 20 insertions(+), 3 deletions(-)
It would be really nice if we could get curl support added to
BlockdevOptionsBase, so that the QMP command for hot-plugging a curl
drive could also control this option. (Hmm, I wonder why curl is
omitted from the list of TODOs in qapi/block-core.json under
BlockdevOptionsBase).
> @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"@}'
Since you are parsing curltimeout as a QEMU_OPT_NUMBER, it should be
"file.curltimeout":10, not "file.curltimeout":"10".
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-12 15:36 ` Eric Blake
@ 2014-08-12 16:04 ` Daniel H Barboza
0 siblings, 0 replies; 8+ messages in thread
From: Daniel H Barboza @ 2014-08-12 16:04 UTC (permalink / raw)
To: Eric Blake, Qemu Devel; +Cc: Kevin Wolf, Stefan Hajnoczi
On 08/12/2014 12:36 PM, Eric Blake wrote:
> On 08/12/2014 08:35 AM, 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 <danielhb@linux.vnet.ibm.com>
>> ---
>> block/curl.c | 13 ++++++++++++-
>> qemu-options.hx | 10 ++++++++--
>> 2 files changed, 20 insertions(+), 3 deletions(-)
> It would be really nice if we could get curl support added to
> BlockdevOptionsBase, so that the QMP command for hot-plugging a curl
> drive could also control this option. (Hmm, I wonder why curl is
> omitted from the list of TODOs in qapi/block-core.json under
> BlockdevOptionsBase).
>
>> @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"@}'
> Since you are parsing curltimeout as a QEMU_OPT_NUMBER, it should be
> "file.curltimeout":10, not "file.curltimeout":"10".
Good catch. I'll fix it in v2.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-12 14:35 [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option Daniel Henrique Barboza
2014-08-12 15:36 ` Eric Blake
@ 2014-08-13 9:15 ` Markus Armbruster
2014-08-13 12:39 ` Daniel H Barboza
1 sibling, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2014-08-13 9:15 UTC (permalink / raw)
To: Daniel Henrique Barboza; +Cc: Kevin Wolf, Qemu Devel, Stefan Hajnoczi
Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> 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 <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..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".
>
> 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.
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-13 9:15 ` Markus Armbruster
@ 2014-08-13 12:39 ` Daniel H Barboza
2014-08-13 13:28 ` Daniel H Barboza
0 siblings, 1 reply; 8+ messages in thread
From: Daniel H Barboza @ 2014-08-13 12:39 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Kevin Wolf, Qemu Devel, Stefan Hajnoczi
On 08/13/2014 06:15 AM, Markus Armbruster wrote:
> Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> 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 <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..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!
>>
>> 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.
>
> [...]
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-13 12:39 ` Daniel H Barboza
@ 2014-08-13 13:28 ` Daniel H Barboza
2014-08-13 14:07 ` Markus Armbruster
0 siblings, 1 reply; 8+ messages in thread
From: Daniel H Barboza @ 2014-08-13 13:28 UTC (permalink / raw)
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 <danielhb@linux.vnet.ibm.com> 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 <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..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.
>>
>> [...]
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-13 13:28 ` Daniel H Barboza
@ 2014-08-13 14:07 ` Markus Armbruster
2014-08-13 14:17 ` Daniel H Barboza
0 siblings, 1 reply; 8+ messages in thread
From: Markus Armbruster @ 2014-08-13 14:07 UTC (permalink / raw)
To: Daniel H Barboza; +Cc: Kevin Wolf, Qemu Devel, Stefan Hajnoczi
Daniel H Barboza <danielhb@linux.vnet.ibm.com> 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 <danielhb@linux.vnet.ibm.com> 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 <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..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".
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option
2014-08-13 14:07 ` Markus Armbruster
@ 2014-08-13 14:17 ` Daniel H Barboza
0 siblings, 0 replies; 8+ messages in thread
From: Daniel H Barboza @ 2014-08-13 14:17 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Kevin Wolf, Qemu Devel, Stefan Hajnoczi
On 08/13/2014 11:07 AM, Markus Armbruster wrote:
> Daniel H Barboza <danielhb@linux.vnet.ibm.com> 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 <danielhb@linux.vnet.ibm.com> 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 <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..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!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-08-13 14:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-12 14:35 [Qemu-devel] [PATCH] block.curl: adding 'curltimeout' option Daniel Henrique Barboza
2014-08-12 15:36 ` Eric Blake
2014-08-12 16:04 ` Daniel H Barboza
2014-08-13 9:15 ` Markus Armbruster
2014-08-13 12:39 ` Daniel H Barboza
2014-08-13 13:28 ` Daniel H Barboza
2014-08-13 14:07 ` Markus Armbruster
2014-08-13 14:17 ` Daniel H Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).