Re: [Qemu-devel] [PATCH 1/4] block/curl: Use BDRV_SECTOR_SIZE

[Richard W.M. Jones]

On Tue, Oct 25, 2016 at 04:54:28AM +0200, Max Reitz wrote:
> Currently, curl defines its own constant SECTOR_SIZE. There is no
> advantage over using the global BDRV_SECTOR_SIZE, so drop it.
> 
> Cc: address@hidden
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/curl.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/block/curl.c b/block/curl.c
> index e5eaa7b..12afa15 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -73,7 +73,6 @@ static CURLMcode __curl_multi_socket_action(CURLM 
> *multi_handle,
>  
>  #define CURL_NUM_STATES 8
>  #define CURL_NUM_ACB    8
> -#define SECTOR_SIZE     512
>  #define READ_AHEAD_DEFAULT (256 * 1024)
>  #define CURL_TIMEOUT_DEFAULT 5
>  #define CURL_TIMEOUT_MAX 10000
> @@ -738,12 +737,12 @@ static void curl_readv_bh_cb(void *p)
>      CURLAIOCB *acb = p;
>      BDRVCURLState *s = acb->common.bs->opaque;
>  
> -    size_t start = acb->sector_num * SECTOR_SIZE;
> +    size_t start = acb->sector_num * BDRV_SECTOR_SIZE;
>      size_t end;
>  
>      // In case we have the requested data already (e.g. read-ahead),
>      // we can just call the callback and be done.
> -    switch (curl_find_buf(s, start, acb->nb_sectors * SECTOR_SIZE, acb)) {
> +    switch (curl_find_buf(s, start, acb->nb_sectors * BDRV_SECTOR_SIZE, 
> acb)) {
>          case FIND_RET_OK:
>              qemu_aio_unref(acb);
>              // fall through
> @@ -762,7 +761,7 @@ static void curl_readv_bh_cb(void *p)
>      }
>  
>      acb->start = 0;
> -    acb->end = (acb->nb_sectors * SECTOR_SIZE);
> +    acb->end = (acb->nb_sectors * BDRV_SECTOR_SIZE);
>  
>      state->buf_off = 0;
>      g_free(state->orig_buf);
> @@ -779,8 +778,8 @@ static void curl_readv_bh_cb(void *p)
>      state->acb[0] = acb;
>  
>      snprintf(state->range, 127, "%zd-%zd", start, end);
> -    DPRINTF("CURL (AIO): Reading %d at %zd (%s)\n",
> -            (acb->nb_sectors * SECTOR_SIZE), start, state->range);
> +    DPRINTF("CURL (AIO): Reading %llu at %zd (%s)\n",
> +            (acb->nb_sectors * BDRV_SECTOR_SIZE), start, state->range);
>      curl_easy_setopt(state->curl, CURLOPT_RANGE, state->range);
>  
>      curl_multi_add_handle(s->multi, state->curl);

ACK, this change is obvious.

Eric's comment about use of size_t instead of off_t or int64_t in
curl_readv_bh_cb (and maybe elsewhere) is dead right too.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/