Re: [Qemu-devel] [RFC PATCH v2 01/23] qcow2: Handle dependencies earlier

[Stefan Hajnoczi]

On Wed, Feb 13, 2013 at 02:21:51PM +0100, Kevin Wolf wrote:
> @@ -882,20 +876,55 @@ int qcow2_alloc_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>      trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset,
>                                        n_start, n_end);
>  
> -    /* Find L2 entry for the first involved cluster */
>  again:
> -    ret = get_cluster_table(bs, offset, &l2_table, &l2_index);
> -    if (ret < 0) {
> -        return ret;
> -    }
> -
>      /*
>       * Calculate the number of clusters to look for. We stop at L2 table
>       * boundaries to keep things simple.
>       */
> +    l2_index = offset_to_l2_index(s, offset);
>      nb_clusters = MIN(size_to_clusters(s, n_end << BDRV_SECTOR_BITS),
>                        s->l2_size - l2_index);
>  
> +    /*
> +     * Now start gathering as many contiguous clusters as possible:
> +     *
> +     * 1. Check for overlaps with in-flight allocations
> +     *
> +     *      a) Overlap not in the first cluster -> shorten this request and 
> let
> +     *         the caller handle the rest in its next loop iteration.
> +     *
> +     *      b) Real overlaps of two requests. Yield and restart the search 
> for
> +     *         contiguous clusters (the situation could have changed while we
> +     *         were sleeping)
> +     *
> +     *      c) TODO: Request starts in the same cluster as the in-flight
> +     *         allocation ends. Shorten the COW of the in-fight allocation, 
> set
> +     *         cluster_offset to write to the same cluster and set up the 
> right
> +     *         synchronisation between the in-flight request and the new one.
> +     *
> +     * 2. Count contiguous COPIED clusters.
> +     *    TODO: Consider cluster_offset if set in step 1c.
> +     *
> +     * 3. If the request still hasn't completed, allocate new clusters,
> +     *    considering any cluster_offset of steps 1c or 2.
> +     */
> +    ret = handle_dependencies(bs, offset, &nb_clusters);
> +    if (ret == -EAGAIN) {
> +        goto again;

Now we might be able to replace the again label with a while loop.

do {
    l2_index = offset_to_l2_index(s, offset);
    nb_clusters = MIN(size_to_clusters(s, n_end << BDRV_SECTOR_BITS),
                      s->l2_size - l2_index);
} while ((ret = handle_dependencies(bs, offset, &nb_clusters)) == -EAGAIN);

Not essential, but perhaps clearer.