| Message ID | 1398882243-14783-29-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hello, sorry for another late detection of the use of too recent features. On Wed, Apr 30, 2014 at 8:24 PM, Kevin Wolf <kwolf@redhat.com> wrote: > From: Matthew Booth <mbooth@redhat.com> > > Remove calls to curl_multi_do where the relevant handles are already > registered to the event loop. > > Ensure that we kick off socket handling with CURL_SOCKET_TIMEOUT after > adding a new handle. > > Signed-off-by: Matthew Booth <mbooth@redhat.com> > Tested-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block/curl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/block/curl.c b/block/curl.c > index 50bd05f..fd2756e 100644 > --- a/block/curl.c > +++ b/block/curl.c > @@ -535,7 +535,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, > curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s); > curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb); > #endif > - curl_multi_do(s); > > qemu_opts_del(opts); > return 0; > @@ -564,6 +563,7 @@ static const AIOCBInfo curl_aiocb_info = { > static void curl_readv_bh_cb(void *p) > { > CURLState *state; > + int running; > > CURLAIOCB *acb = p; > BDRVCURLState *s = acb->common.bs->opaque; > @@ -612,8 +612,9 @@ static void curl_readv_bh_cb(void *p) > curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range); > > curl_multi_add_handle(s->multi, state->curl); > - curl_multi_do(s); > > + /* Tell curl it needs to kick things off */ > + curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running); curl_multi_socket_action isn't available on the CURL lib used by CentOS 5.6. The obvious workaround is to disable CURL when calling configure. Thanks, Laurent > } > > static BlockDriverAIOCB *curl_aio_readv(BlockDriverState *bs, > -- > 1.8.3.1 > >
Am 06.05.2014 um 11:44 hat Laurent Desnogues geschrieben: > Hello, > > sorry for another late detection of the use of too recent features. > > On Wed, Apr 30, 2014 at 8:24 PM, Kevin Wolf <kwolf@redhat.com> wrote: > > From: Matthew Booth <mbooth@redhat.com> > > > > Remove calls to curl_multi_do where the relevant handles are already > > registered to the event loop. > > > > Ensure that we kick off socket handling with CURL_SOCKET_TIMEOUT after > > adding a new handle. > > > > Signed-off-by: Matthew Booth <mbooth@redhat.com> > > Tested-by: Richard W.M. Jones <rjones@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > block/curl.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/block/curl.c b/block/curl.c > > index 50bd05f..fd2756e 100644 > > --- a/block/curl.c > > +++ b/block/curl.c > > @@ -535,7 +535,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, > > curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s); > > curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb); > > #endif > > - curl_multi_do(s); > > > > qemu_opts_del(opts); > > return 0; > > @@ -564,6 +563,7 @@ static const AIOCBInfo curl_aiocb_info = { > > static void curl_readv_bh_cb(void *p) > > { > > CURLState *state; > > + int running; > > > > CURLAIOCB *acb = p; > > BDRVCURLState *s = acb->common.bs->opaque; > > @@ -612,8 +612,9 @@ static void curl_readv_bh_cb(void *p) > > curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range); > > > > curl_multi_add_handle(s->multi, state->curl); > > - curl_multi_do(s); > > > > + /* Tell curl it needs to kick things off */ > > + curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running); > > curl_multi_socket_action isn't available on the CURL lib used > by CentOS 5.6. The obvious workaround is to disable CURL > when calling configure. Matthew, can you please check whether this call can be avoided, or otherwise change configure to check the presence of the function? Kevin
On 06/05/14 10:56, Kevin Wolf wrote: > Am 06.05.2014 um 11:44 hat Laurent Desnogues geschrieben: >> Hello, >> >> sorry for another late detection of the use of too recent features. >> >> On Wed, Apr 30, 2014 at 8:24 PM, Kevin Wolf <kwolf@redhat.com> wrote: >>> From: Matthew Booth <mbooth@redhat.com> >>> >>> Remove calls to curl_multi_do where the relevant handles are already >>> registered to the event loop. >>> >>> Ensure that we kick off socket handling with CURL_SOCKET_TIMEOUT after >>> adding a new handle. >>> >>> Signed-off-by: Matthew Booth <mbooth@redhat.com> >>> Tested-by: Richard W.M. Jones <rjones@redhat.com> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> block/curl.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/block/curl.c b/block/curl.c >>> index 50bd05f..fd2756e 100644 >>> --- a/block/curl.c >>> +++ b/block/curl.c >>> @@ -535,7 +535,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, >>> curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s); >>> curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb); >>> #endif >>> - curl_multi_do(s); >>> >>> qemu_opts_del(opts); >>> return 0; >>> @@ -564,6 +563,7 @@ static const AIOCBInfo curl_aiocb_info = { >>> static void curl_readv_bh_cb(void *p) >>> { >>> CURLState *state; >>> + int running; >>> >>> CURLAIOCB *acb = p; >>> BDRVCURLState *s = acb->common.bs->opaque; >>> @@ -612,8 +612,9 @@ static void curl_readv_bh_cb(void *p) >>> curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range); >>> >>> curl_multi_add_handle(s->multi, state->curl); >>> - curl_multi_do(s); >>> >>> + /* Tell curl it needs to kick things off */ >>> + curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running); >> >> curl_multi_socket_action isn't available on the CURL lib used >> by CentOS 5.6. The obvious workaround is to disable CURL >> when calling configure. > > Matthew, can you please check whether this call can be avoided, or > otherwise change configure to check the presence of the function? I should be able to sort this if I can work out how to add a configure test. When do you need it by? If I'm too slow it's safe to revert this change. It's just a cleanup to remove use of a deprecated api. Matt
Am 06.05.2014 um 14:41 hat Matthew Booth geschrieben: > On 06/05/14 10:56, Kevin Wolf wrote: > > Am 06.05.2014 um 11:44 hat Laurent Desnogues geschrieben: > >> curl_multi_socket_action isn't available on the CURL lib used > >> by CentOS 5.6. The obvious workaround is to disable CURL > >> when calling configure. > > > > Matthew, can you please check whether this call can be avoided, or > > otherwise change configure to check the presence of the function? > > I should be able to sort this if I can work out how to add a configure > test. When do you need it by? If I'm too slow it's safe to revert this > change. It's just a cleanup to remove use of a deprecated api. We have plenty of time until 2.1, so I don't think that's a problem. Anyway, if you need to do the configure change, just search for "curl" in configure. We already probe for some functions, you can just add another call in the program for the test compile. Kevin
diff --git a/block/curl.c b/block/curl.c index 50bd05f..fd2756e 100644 --- a/block/curl.c +++ b/block/curl.c @@ -535,7 +535,6 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s); curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb); #endif - curl_multi_do(s); qemu_opts_del(opts); return 0; @@ -564,6 +563,7 @@ static const AIOCBInfo curl_aiocb_info = { static void curl_readv_bh_cb(void *p) { CURLState *state; + int running; CURLAIOCB *acb = p; BDRVCURLState *s = acb->common.bs->opaque; @@ -612,8 +612,9 @@ static void curl_readv_bh_cb(void *p) curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range); curl_multi_add_handle(s->multi, state->curl); - curl_multi_do(s); + /* Tell curl it needs to kick things off */ + curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running); } static BlockDriverAIOCB *curl_aio_readv(BlockDriverState *bs,