Re: [Qemu-devel] [PATCH] curl: fix curl read

[Fam Zheng]

On Thu, 05/09 08:41, Stefan Hajnoczi wrote:
> On Fri, May 03, 2013 at 04:00:09PM +0800, Fam Zheng wrote:
> > @@ -391,7 +427,12 @@ static QemuOptsList runtime_opts = {
> >              .type = QEMU_OPT_SIZE,
> >              .help = "Readahead size",
> >          },
> > -        { /* end of list */ }
> > +        {
> > +            .name = "ssl_no_cert",
> > +            .type = QEMU_OPT_BOOL,
> > +            .help = "SSL certificate check",
> > +        },
> 
> This new option should be in a separate patch.

OK. I'll try to split them to patches.

> 
> > +    if (!strncmp(s->url, "http://";, strlen("http://";)) && 
> > !s->accept_range) {
> > +        strncpy(state->errmsg, "Server not supporting range.", 
> > CURL_ERROR_SIZE);
> > +        goto out;
> > +    }
> 
> This check is unrelated to the API change and should be in a separate
> patch.
> 
> >      s->multi = curl_multi_init();
> > -    curl_multi_setopt( s->multi, CURLMOPT_SOCKETDATA, s); 
> > -    curl_multi_setopt( s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb ); 
> > -    curl_multi_do(s);
> > +    if (!s->multi) {
> > +        goto out_noclean;
> > +    }
> > +    curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
> > +    curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
> > +    curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
> > +    curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, 
> > curl_multi_timer_cb);
> > +    curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, &running);
> 
> The timeout should be added in a separate patch.
> 
> > +    cache = curl_find_cache(s, aio_base, aio_bytes);
> > +    if (cache) {
> > +        curl_complete_io(s, acb, cache);
> > +        return;
> >      }
> 
> What is the point of the cache?  Can you split it into a separate patch?

The cache is for prefetch. Data is fetched by 256k chunks using libcurl
and stored in cache to fill future io request, reducing overall http
request overhead.

-- 
Fam