RE: [PATCH 13/13] block/qcow2-cluster: make range overlap check more readable

[Xingtao Yao (Fujitsu)]

ping.

> -----Original Message-----
> From: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> Sent: Monday, July 22, 2024 12:08 PM
> To: qemu-devel@nongnu.org; Kevin Wolf <kwolf@redhat.com>; Hanna Reitz
> <hreitz@redhat.com>
> Cc: Yao, Xingtao/姚 幸涛 <yaoxt.fnst@fujitsu.com>; qemu-block@nongnu.org
> Subject: [PATCH 13/13] block/qcow2-cluster: make range overlap check more
> readable
> 
> use range_overlaps_range() instead of open-coding the overlap check to
> improve the readability of the code.
> 
> Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com>
> ---
>  block/qcow2-cluster.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index ce8c0076b3b5..88d65c4b99e6 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -30,6 +30,7 @@
>  #include "qcow2.h"
>  #include "qemu/bswap.h"
>  #include "qemu/memalign.h"
> +#include "qemu/range.h"
>  #include "trace.h"
> 
>  int coroutine_fn qcow2_shrink_l1_table(BlockDriverState *bs,
> @@ -1408,23 +1409,25 @@ static int coroutine_fn
> handle_dependencies(BlockDriverState *bs,
>      BDRVQcow2State *s = bs->opaque;
>      QCowL2Meta *old_alloc;
>      uint64_t bytes = *cur_bytes;
> +    Range range1, range2, range3;
> 
> +    range_init_nofail(&range1, guest_offset, bytes);
>      QLIST_FOREACH(old_alloc, &s->cluster_allocs, next_in_flight) {
> 
> -        uint64_t start = guest_offset;
> -        uint64_t end = start + bytes;
> -        uint64_t old_start = start_of_cluster(s, 
> l2meta_cow_start(old_alloc));
> -        uint64_t old_end = ROUND_UP(l2meta_cow_end(old_alloc),
> s->cluster_size);
> +        uint64_t cow_start = l2meta_cow_start(old_alloc);
> +        uint64_t cow_end = l2meta_cow_end(old_alloc);
> +        uint64_t start = start_of_cluster(s, cow_start);
> +        uint64_t end = ROUND_UP(cow_end, s->cluster_size);
> 
> -        if (end <= old_start || start >= old_end) {
> +        range_init_nofail(&range2, start, end - start);
> +        if (!range_overlaps_range(&range1, &range2)) {
>              /* No intersection */
>              continue;
>          }
> 
> +        range_init_nofail(&range3, cow_start, cow_end - cow_start);
>          if (old_alloc->keep_old_clusters &&
> -            (end <= l2meta_cow_start(old_alloc) ||
> -             start >= l2meta_cow_end(old_alloc)))
> -        {
> +            !range_overlaps_range(&range1, &range3)) {
>              /*
>               * Clusters intersect but COW areas don't. And cluster itself is
>               * already allocated. So, there is no actual conflict.
> @@ -1434,9 +1437,9 @@ static int coroutine_fn
> handle_dependencies(BlockDriverState *bs,
> 
>          /* Conflict */
> 
> -        if (start < old_start) {
> +        if (guest_offset < start) {
>              /* Stop at the start of a running allocation */
> -            bytes = old_start - start;
> +            bytes = start - guest_offset;
>          } else {
>              bytes = 0;
>          }
> --
> 2.41.0