From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHZhX-0003Bg-H2 for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:39:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XHZhS-0004zD-Ck for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:38:55 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:52829 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XHZhS-0004z8-3b for qemu-devel@nongnu.org; Wed, 13 Aug 2014 10:38:50 -0400 Date: Wed, 13 Aug 2014 16:38:00 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140813143800.GC3091@irqsave.net> References: <1407870777-594-1-git-send-email-danielhb@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1407870777-594-1-git-send-email-danielhb@linux.vnet.ibm.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2] block.curl: adding 'curltimeout' option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Daniel Henrique Barboza Cc: Qemu Devel The Tuesday 12 Aug 2014 =E0 16:12:57 (-0300), Daniel Henrique Barboza wro= te : > 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. >=20 > 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. >=20 > Signed-off-by: Daniel Henrique Barboza > --- > block/curl.c | 13 ++++++++++++- > qemu-options.hx | 10 ++++++++-- > 2 files changed, 20 insertions(+), 3 deletions(-) >=20 > 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 *mu= lti_handle, > #define CURL_NUM_ACB 8 > #define SECTOR_SIZE 512 > #define READ_AHEAD_DEFAULT (256 * 1024) > +#define CURL_TIMEOUT_DEFAULT 5 > =20 > #define FIND_RET_NONE 0 > #define FIND_RET_OK 1 > @@ -71,6 +72,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *mu= lti_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" > =20 > struct BDRVCURLState; > =20 > @@ -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 n= ot name the field curl_timeout like it would be in every other part of qemu = code ? > 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 =3D { > .type =3D QEMU_OPT_BOOL, > .help =3D "Verify SSL certificate" > }, > + { > + .name =3D CURL_BLOCK_OPT_TIMEOUT, > + .type =3D QEMU_OPT_NUMBER, > + .help =3D "Curl timeout" > + }, > { /* end of list */ } > }, > }; > @@ -525,6 +533,9 @@ static int curl_open(BlockDriverState *bs, QDict *o= ptions, int flags, > goto out_noclean; > } > =20 > + s->curltimeout =3D qemu_opt_get_number(opts, CURL_BLOCK_OPT_TIMEOU= T, > + CURL_TIMEOUT_DEFAULT); > + > s->sslverify =3D qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY,= true); > =20 > file =3D 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 i= s used. > @end table > =20 > Note that when passing options to qemu explicitly, @option{driver} is = the value > @@ -2372,9 +2377,10 @@ qemu-system-x86_64 -drive file=3D/tmp/Fedora-x86= _64-20-20131211.1-sda.qcow2,copy-o > @end example > =20 > Example: boot from an image stored on a VMware vSphere server with a s= elf-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=3D'json:@{"file.driver":"http= s",, "file.url":"https://user:password@@vsphere.example.com/folder/test/t= est-flat.vmdk?dcPath=3DDatacenter&dsName=3Ddatastore1",, "file.sslverify"= :"off",, "file.readahead":"64k"@}' /tmp/test.qcow2 > +qemu-img create -f qcow2 -o backing_file=3D'json:@{"file.driver":"http= s",, "file.url":"https://user:password@@vsphere.example.com/folder/test/t= est-flat.vmdk?dcPath=3DDatacenter&dsName=3Ddatastore1",, "file.sslverify"= :"off",, "file.readahead":"64k",, "file.curltimeout":10@}' /tmp/test.qcow= 2 > =20 > qemu-system-x86_64 -drive file=3D/tmp/test.qcow2 > @end example > --=20 > 1.8.3.1 >=20 >=20 Apart from the coding style issue. Reviewed-by: Benoit Canet