Re: [RFC PATCH v3 25/27] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

[Eric Blake <eblake@redhat.com>]

On 12/22/19 5:37 AM, Alberto Garcia wrote:
> Now that the implementation of subclusters is complete we can finally
> add the necessary options to create and read images with this feature,
> which we call "extended L2 entries".
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---

Preliminary review on just interface items (I may do a deeper dive into 
the rest of the patch after getting through the series).

> +++ b/block/qcow2.c
> @@ -1383,6 +1383,12 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
>       s->subcluster_size = s->cluster_size / s->subclusters_per_cluster;
>       s->subcluster_bits = ctz32(s->subcluster_size);
>   
> +    if (s->subcluster_size < (1 << MIN_CLUSTER_BITS)) {
> +        error_setg(errp, "Unsupported subcluster size: %d", s->subcluster_size);
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
>       /* Check support for various header values */
>       if (header.refcount_order > 6) {
>           error_setg(errp, "Reference count entry width too large; may not "
> @@ -2856,6 +2862,11 @@ int qcow2_update_header(BlockDriverState *bs)
>                   .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
>                   .name = "lazy refcounts",
>               },
> +            {
> +                .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
> +                .bit  = QCOW2_INCOMPAT_EXTL2_BITNR,
> +                .name = "extended L2 entries",
> +            },

I'd sort this to be grouped with the other INCOMPATIBLE bits (after 
"external data file", rather than placing a COMPATIBLE bit in the middle.

Rebase conflict with my patches proposing the addition of an AUTOCLEAR 
bit, here and in the impacted iotests.  Should be trivial to resolve, by 
whoever lands second.

> +++ b/qapi/block-core.json
> @@ -66,6 +66,9 @@
>   #                 standalone (read-only) raw image without looking at qcow2
>   #                 metadata (since: 4.0)
>   #
> +# @extended-l2: true if the image has extended L2 entries; only valid for
> +#               compat >= 1.1 (since 4.2)
> +#

5.0, now.

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