[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 6/6] qcow2: Allow lazy refcounts to be enabl
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [RFC PATCH 6/6] qcow2: Allow lazy refcounts to be enabled on the command line |
Date: |
Thu, 28 Feb 2013 10:37:18 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
- [Qemu-devel] [RFC PATCH 0/6] block: Add driver specific options, Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 3/6] Add qdict_clone_shallow(), Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 2/6] block: Add options QDict to bdrv_open() prototype, Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 1/6] block: Add options QDict to .bdrv_open(), Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 5/6] block: Support driver specific options in drive_init(), Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 6/6] qcow2: Allow lazy refcounts to be enabled on the command line, Kevin Wolf, 2013/02/27
- [Qemu-devel] [RFC PATCH 4/6] block: Add options QDict to bdrv_open_common(), Kevin Wolf, 2013/02/27
- Re: [Qemu-devel] [RFC PATCH 0/6] block: Add driver specific options, Eric Blake, 2013/02/27
- Re: [Qemu-devel] [RFC PATCH 0/6] block: Add driver specific options, Stefan Hajnoczi, 2013/02/28