Re: [Qemu-block] [Qemu-devel] [PATCH 05/29] qed: Remove callback from qed_find_cluster()

[Eric Blake]

On 05/26/2017 03:21 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block/qed-cluster.c | 39 ++++++++++++++++++++++-----------------
>  block/qed.c         | 20 +++++++++++---------
>  block/qed.h         |  4 ++--
>  3 files changed, 35 insertions(+), 28 deletions(-)
> 

This diffstat is not as slick as the earlier patches, but the cleanup is
still worthwhile.

> @@ -93,16 +98,16 @@ void qed_find_cluster(BDRVQEDState *s, QEDRequest 
> *request, uint64_t pos,
>      /* Limit length to L2 boundary.  Requests are broken up at the L2 
> boundary
>       * so that a request acts on one L2 table at a time.
>       */
> -    len = MIN(len, (((pos >> s->l1_shift) + 1) << s->l1_shift) - pos);
> +    *len = MIN(*len, (((pos >> s->l1_shift) + 1) << s->l1_shift) - pos);

Perhaps a separate cleanup, but wouldn't this be easier to read as:
*len = MIN(*len, QEMU_ALIGN_UP(pos, 1ULL << s->l1_shift) - pos);

(or if there is a direct variable for l1 size, rather than '1ULL <<
s->l1_shift')

> +++ b/block/qed.h
> @@ -247,8 +247,8 @@ int qed_write_l2_table_sync(BDRVQEDState *s, QEDRequest 
> *request,
>  /**
>   * Cluster functions
>   */
> -void qed_find_cluster(BDRVQEDState *s, QEDRequest *request, uint64_t pos,
> -                      size_t len, QEDFindClusterFunc *cb, void *opaque);
> +int qed_find_cluster(BDRVQEDState *s, QEDRequest *request, uint64_t pos,
> +                     size_t *len, uint64_t *img_offset);

While we're touching this, would it be better to switch to an
always-64-bit type, rather than using platform-dependent size_t for len?
 But even so, that cleanup would be a separate patch.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature