Re: [Qemu-devel] [PATCH v2 01/12] qcow2: Add new overlap check functions

[Stefan Hajnoczi]

On Mon, Nov 24, 2014 at 04:56:49PM +0100, Max Reitz wrote:
> +static void build_window_bitmap(Qcow2MetadataList *mdl,
> +                                Qcow2MetadataWindow *window)
> +{
> +    int cache_i, oldest_cache_i = -1, i;
> +    unsigned oldest_cache_age = 0;
> +
> +    for (cache_i = 0; cache_i < mdl->nb_cached_windows; cache_i++) {
> +        unsigned age;
> +
> +        if (mdl->cached_windows[cache_i] < 0) {
> +            break;
> +        }
> +
> +        age = mdl->current_age - 
> mdl->windows[mdl->cached_windows[cache_i]].age;
> +        if (age > oldest_cache_age) {
> +            oldest_cache_age = age;
> +            oldest_cache_i = cache_i;
> +        }
> +    }
> +
> +    if (cache_i >= mdl->nb_cached_windows) {
> +        destroy_window_bitmap(mdl,
> +            &mdl->windows[mdl->cached_windows[oldest_cache_i]]);
> +        cache_i = oldest_cache_i;
> +    }
> +
> +    assert(cache_i >= 0);
> +    mdl->cached_windows[cache_i] = window - mdl->windows;
> +    window->cached_windows_index = cache_i;

Is this field ever used?

> +/**
> + * Removes a range of the given types from the metadata list.
> + */
> +void qcow2_metadata_list_remove(BlockDriverState *bs, uint64_t offset,
> +                                int nb_clusters, QCow2MetadataOverlap types)
> +{
> +    BDRVQcowState *s = bs->opaque;
> +    uint64_t start_cluster = offset >> s->cluster_bits;
> +    uint64_t end_cluster = start_cluster + nb_clusters;
> +    uint64_t current_cluster = start_cluster;
> +
> +    types &= s->overlap_check;
> +    if (!types) {
> +        return;
> +    }
> +
> +    if (offset_into_cluster(s, offset)) {
> +        /* Try to remove even broken metadata ranges */
> +        end_cluster++;

Why does it make sense to go ahead (and add another cluster on to the
end) when offset is not cluster-aligned?

It seems we might clear out the wrong metadata.

pgpZ0UdR6ta6V.pgp
Description: PGP signature