Re: [Qemu-devel] [PATCH v5 06/11] curl: introduce CURLDataCache

[Stefan Hajnoczi <stefanha@gmail.com>]

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!

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

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

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