Re: [Qemu-devel] [PATCH 3/5] qcow2: Options to enable discard for freed clusters

[Kevin Wolf]

Am 14.06.2013 um 00:10 hat Paolo Bonzini geschrieben:
> Il 13/06/2013 07:47, Kevin Wolf ha scritto:
> > +    s->discard_passthrough[QCOW2_DISCARD_NEVER] = false,
> > +    s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true,
> > +    s->discard_passthrough[QCOW2_DISCARD_REQUEST] =
> > +        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST,
> > +                          flags & BDRV_O_UNMAP),
> 
> I think there should not be two ways to enable it, it is confusing.

Hm, yes... But it's also confusing to have qcow2 provide an incomplete
set of categories. Maybe we shouldn't have introduced -drive discard=...
as a global option to begin with.

> > +    s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] =
> > +        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true),
> > +    s->discard_passthrough[QCOW2_DISCARD_OTHER] =
> > +        qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false),
> 
> Please document the defaults in qcow2_runtime_opts.  (BTW, what is the
> rationale?)

The idea was that discard is slow and therefore disabled by default,
except when you're doing an expensive snapshot operation that can
potentially free a lot of space at once with not too many requests, so
there it's enabled. And if you said -drive discard=on, you obviously
want guest requests to take effect.

We could let QCOW2_OPT_DISCARD_OTHER default to BDRV_O_UNMAP as well if
you prefer.

Kevin