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

[Kevin Wolf]

Am 28.02.2013 um 04:10 hat Eric Blake geschrieben:
> 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)?

Yes, this is a bug. I hope this manual parsing code will go away anyway,
so this was mostly so I could demonstrate that the option passing was
working.

> > +        }
> > +        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?

Good point, I should have checked this.

Kevin