Re: [Qemu-devel] [PULL 00/31] Block patches

Re: [Qemu-devel] [PULL 00/31] Block patches

[Peter Maydell]

On 30 April 2014 19:23, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit e2da502c003b9a91b4aea7684959192bd07c1f1d:
>
>   Merge remote-tracking branch 'remotes/otubo/seccomp' into staging (2014-04-28 14:14:35 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to b7079df4100069959f4e9d90d5cb5ba7d4ebbf1a:
>
>   curl: Fix hang reading from slow connections (2014-04-30 16:34:21 +0200)
>
> ----------------------------------------------------------------
> Block patches

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 16/31] block: Fix open_flags in bdrv_reopen()

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

On 2014-04-30 20:23, Kevin Wolf wrote:
> Use the same function as bdrv_open() for determining what the right
> flags for bs->file are. Without doing this, a reopen means that
> bs->file loses BDRV_O_CACHE_WB or BDRV_O_UNMAP if bs doesn't have it as
> well.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 9f6f07e..b749d31 100644
> --- a/block.c
> +++ b/block.c
> @@ -1525,8 +1525,11 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
>          QSIMPLEQ_INIT(bs_queue);
>      }
>  
> +    /* bdrv_open() masks this flag out */
> +    flags &= ~BDRV_O_PROTOCOL;
> +
>      if (bs->file) {
> -        bdrv_reopen_queue(bs_queue, bs->file, flags);
> +        bdrv_reopen_queue(bs_queue, bs->file, bdrv_inherited_flags(flags));
>      }
>  
>      bs_entry = g_new0(BlockReopenQueueEntry, 1);
> 

This breaks the "commit" monitor command for disks in snapshot mode.
Returned error is "no permission" (I assume -EPERM).

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [Qemu-devel] [PULL 28/31] curl: Remove unnecessary explicit calls to internal event handler

[Laurent Desnogues]

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
>
>

Re: [Qemu-devel] [PULL 28/31] curl: Remove unnecessary explicit calls to internal event handler

[Kevin Wolf]

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

Re: [Qemu-devel] [PULL 28/31] curl: Remove unnecessary explicit calls to internal event handler

[Matthew Booth]

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
-- 
Matthew Booth
Red Hat Engineering, Virtualisation Team

Phone: +442070094448 (UK)
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

Re: [Qemu-devel] [PULL 28/31] curl: Remove unnecessary explicit calls to internal event handler

[Kevin Wolf]

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