Re: [PATCH 2/3] qcow2: refactor handle_dependencies() loop body

[Eric Blake <eblake@redhat.com>]

On Sat, Jul 24, 2021 at 04:38:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> No logic change, just prepare for the following commit.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2-cluster.c | 49 ++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 21 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index bd0597842f..967121c7e6 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1400,29 +1400,36 @@ static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
>  
>          if (end <= old_start || start >= old_end) {
>              /* No intersection */
> -        } else {
> -            if (start < old_start) {
> -                /* Stop at the start of a running allocation */
> -                bytes = old_start - start;
> -            } else {
> -                bytes = 0;
> -            }
> +            continue;
> +        }
>  
> -            /* Stop if already an l2meta exists. After yielding, it wouldn't

Pre-existing, but...

> -             * be valid any more, so we'd have to clean up the old L2Metas
> -             * and deal with requests depending on them before starting to
> -             * gather new ones. Not worth the trouble. */
> -            if (bytes == 0 && *m) {
> -                *cur_bytes = 0;
> -                return 0;
> -            }
> +        /* Conflict */
>  
> -            if (bytes == 0) {
> -                /* Wait for the dependency to complete. We need to recheck
> -                 * the free/allocated clusters when we continue. */
> -                qemu_co_queue_wait(&old_alloc->dependent_requests, &s->lock);
> -                return -EAGAIN;
> -            }
> +        if (start < old_start) {
> +            /* Stop at the start of a running allocation */
> +            bytes = old_start - start;
> +        } else {
> +            bytes = 0;
> +        }
> +
> +        /*
> +         * Stop if already an l2meta exists. After yielding, it wouldn't

...might as well fix the grammar:  Stop if an l2meta already exists.

> +         * be valid any more, so we'd have to clean up the old L2Metas
> +         * and deal with requests depending on them before starting to
> +         * gather new ones. Not worth the trouble.
> +         */
> +        if (bytes == 0 && *m) {
> +            *cur_bytes = 0;
> +            return 0;
> +        }
> +
> +        if (bytes == 0) {
> +            /*
> +             * Wait for the dependency to complete. We need to recheck
> +             * the free/allocated clusters when we continue.
> +             */
> +            qemu_co_queue_wait(&old_alloc->dependent_requests, &s->lock);
> +            return -EAGAIN;

So the change adds a short-circuiting 'continue', then reduces the
indentation of the rest of the loop body.

Reviewed-by: Eric Blake <eblake@redhat.com>

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