Re: [Qemu-devel] [PATCH v3 3/5] qcow2: Introduce an option for sufficient L2 cache for the entire image

[Kevin Wolf <kwolf@redhat.com>]

Am 25.07.2018 um 14:32 hat Leonid Bloch geschrieben:
> On 07/25/2018 03:22 PM, Eric Blake wrote:
> 
>     On 07/25/2018 03:26 AM, Kevin Wolf wrote:
> 
>         Only looking at the external interface for now, I wonder whether it
>         would be nicer not to have two mutually exclusive options, but to make
>         l2-cache-size an alternate that can take either an int like before
>         (meaning the number of bytes) or a string/enum (with the only accepted
>         value "full" for now).
> 
>     That does sound interesting.
> 
> This does, but currently QEMU supports QEMU_OPT_STRING, QEMU_OPT_BOOL,
> QEMU_OPT_NUMBER, and QEMU_OPT_SIZE. Looks like it will require a more
> fundamental change to accept an option that can be either a string or a size.

Hm, yes, good point. We wouldn't be able to parse the options purely
with QemuOpts any more. So we would have to manually check for 'full' in
the QDict before calling qemu_opts_absorb_qdict(). If it's there, we
would have to process it and then delete it from the QDict before we
feed the QDict to qemu_opts_absorb_qdict(), which would only accept a
number there. A bit ugly, but should be workable.

Maybe this is really the time that we should convert qcow2 to use the
QAPI types anyway, like some of the protocol drivers do internally now.
Obviously, this is out of scope for this series, but it gives a
perspective for how to get rid of the ugliness again.

>         Another interesting question is whether 'full' shouldn't keep meaning
>         full throughout the lifetime of the BlockDriverState, i.e. should it
>         keep adapting to the new size when the image size changes?
> 
> 
>     Do we even resize the cache now for image size changes? If we use an enum,
>     we could have two different values depending on whether the chosen cache
>     size remains fixed or also tries to resize when the image grows.

We don't because we only support absolute cache sizes today. 'full'
would be the first one that is relative to the image size.

> Is it even possible to change the virtual disk image size online?

Yes, this is what qcow2_co_truncate() does (can be invoked, amongst
others, with the QMP command 'block_resize').

> Found a problem with my previous patch: the property was not actually set as a
> proper boolean option. Also, fixing the error output in iotest 103 (thanks
> Kevin for the catch!). V5 is on the way.

Maybe give the alternate thing a try with v5, as everyone seems to agree
that it's a nicer interface if we can make it work.

Also, a meta-comment: Leonid, would you mind sending plain text emails
instead of HTML-only?

Kevin