On Thu, 05/23 16:09, Stefan Hajnoczi wrote:
> On Thu, May 23, 2013 at 11:38:04AM +0800, Fam Zheng wrote:
> > +typedef struct CURLDataCache {
> > + char *data;
> > + size_t base_pos;
>
> Must be int64_t. QEMU compiled on 32-bit hosts would only allow 4 GB
> images with size_t!
OK.
>
> > + size_t data_len;
> > + size_t write_pos;
> > + /* Ref count for CURLState */
> > + int use_count;
>
> It's better to introduce this field when you add code to use it. When
> possible, don't add unused code in a patch, it makes it harder to
> review.
Moving to later patch.
>
> > +static void curl_complete_io(BDRVCURLState *bs, CURLAIOCB *acb,
> > + CURLDataCache *cache)
> > +{
> > + size_t aio_base = acb->sector_num * SECTOR_SIZE;
>
> int64_t
>
> > + size_t aio_bytes = acb->nb_sectors * SECTOR_SIZE;
> > + size_t off = aio_base - cache->base_pos;
> > +
> > + qemu_iovec_from_buf(acb->qiov, 0, cache->data + off, aio_bytes);
> > + acb->common.cb(acb->common.opaque, 0);
> > + DPRINTF("AIO Request OK: %10zd %10zd\n", aio_base, aio_bytes);
>
> PRId64 for 64-bit aio_base
OK, thanks.
>
> > @@ -589,26 +577,24 @@ static const AIOCBInfo curl_aiocb_info = {
> > static void curl_readv_bh_cb(void *p)
> > {
> > CURLState *state;
> > -
> > + CURLDataCache *cache = NULL;
> > CURLAIOCB *acb = p;
> > BDRVCURLState *s = acb->common.bs->opaque;
> > + size_t aio_base, aio_bytes;
>
> int64_t aio_base;
Yes, will change.
--
Fam
