Re: [Qemu-block] [PATCH v4 04/10] qcow2: Implement copy offloading

[Stefan Hajnoczi]

On Fri, May 11, 2018 at 08:08:17PM +0800, Fam Zheng wrote:
> +static int qcow2_handle_l2meta(BlockDriverState *bs, QCowL2Meta *l2meta)
> +{
> +    int ret = 0;
> +
> +    while (l2meta != NULL) {
> +        QCowL2Meta *next;
> +
> +        if (!ret) {
> +            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> +        }
> +
> +        /* Take the request off the list of running requests */
> +        if (l2meta->nb_clusters != 0) {
> +            QLIST_REMOVE(l2meta, next_in_flight);
> +        }
> +
> +        qemu_co_queue_restart_all(&l2meta->dependent_requests);

A coroutine_fn may only be called by a coroutine_fn.  Please mark
qcow2_handle_l2meta() coroutine_fn.

> @@ -2069,18 +2080,7 @@ static coroutine_fn int 
> qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
>      ret = 0;
>  
>  fail:
> -    while (l2meta != NULL) {
> -        QCowL2Meta *next;
> -
> -        if (l2meta->nb_clusters != 0) {
> -            QLIST_REMOVE(l2meta, next_in_flight);
> -        }
> -        qemu_co_queue_restart_all(&l2meta->dependent_requests);
> -
> -        next = l2meta->next;
> -        g_free(l2meta);
> -        l2meta = next;
> -    }
> +    qcow2_handle_l2meta(bs, l2meta);

Is qcow2_handle_l2meta() equivalent to the code that is being removed?
qcow2_handle_l2meta() calls qcow2_alloc_cluster_link_l2() while this
code does not.

>  
>      qemu_co_mutex_unlock(&s->lock);
>  
> @@ -3267,6 +3267,176 @@ static coroutine_fn int 
> qcow2_co_pdiscard(BlockDriverState *bs,
>      return ret;
>  }
>  
> +static int qcow2_co_copy_range_from(BlockDriverState *bs,

Missing coroutine_fn.  Please check your patches for more occurrences.

> +        while (l2meta != NULL) {
> +            QCowL2Meta *next;
> +
> +            ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
> +            if (ret < 0) {
> +                goto fail;
> +            }
> +
> +            /* Take the request off the list of running requests */
> +            if (l2meta->nb_clusters != 0) {
> +                QLIST_REMOVE(l2meta, next_in_flight);
> +            }
> +
> +            qemu_co_queue_restart_all(&l2meta->dependent_requests);
> +
> +            next = l2meta->next;
> +            g_free(l2meta);
> +            l2meta = next;
> +        }

Why isn't this a call to qcow2_handle_l2meta()?  I guess the reason for
extracting that function was to use it here?

signature.asc
Description: PGP signature