Re: [Qemu-devel] [RFC PATCH 6/6] qcow2: Allow lazy refcounts to be enabled on the command line

[Eric Blake]

On 02/27/2013 10:25 AM, Kevin Wolf wrote:
> qcow2 images now accept a boolean lazy_refcouns options. Use it like

s/refcouns/refcounts/

> this:
> 
>   -drive file=test.qcow2,lazy_refcounts=on
> 
> If the option is specified on the comman line, it overrides the default

s/comman/command/

> specified by the qcow2 header flags that were set when creating the
> image.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block/qcow2-cluster.c |  2 +-
>  block/qcow2.c         | 20 ++++++++++++++++++++
>  block/qcow2.h         |  1 +
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 

> +++ b/block/qcow2.c
> @@ -495,6 +495,26 @@ static int qcow2_open(BlockDriverState *bs, QDict 
> *options, int flags)
>          }
>      }
>  
> +    /* Enable lazy_refcounts according to image and command line options */
> +    if (qdict_haskey(options, "lazy_refcounts")) {
> +        const char *value = qdict_get_str(options, "lazy_refcounts");
> +        if (!strcmp(value, "on")) {
> +            s->use_lazy_refcounts = true;
> +        } else if (!strcmp(value, "off")) {
> +            s->use_lazy_refcounts = false;
> +        } else {
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE,
> +                "lazy_refcounts", "'on' or 'off'");
> +            ret = -EINVAL;
> +            goto fail;

If I pass 'lazy_refcounts=foo', it doesn't get deleted from this layer,
and then the caller notices that it is still in the dict and reports a
second error about an unconsumed option.  Shouldn't this layer
unconditionally remove lazy_refcounts from the dict, because we handle
it here (even if our handling is reporting an error about invalid usage)?

> +        }
> +        qdict_del(options, "lazy_refcounts");
> +    } else {
> +        s->use_lazy_refcounts =
> +            (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS);
> +    }

What happens if I pass in a qcow2 file that does not support qcow2v3
options (compat=0.10), but then ask for lazy_refcounts=on?  Should that
be flagged as an error, because lazy refcounts only work if you have a
compat=1.1 image?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature