On 07/25/2018 04:32 PM, Kevin Wolf wrote:
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.
I need to look into that. Thanks for the idea. But indeed looks like out
of scope for this series.
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').
Cool! This does look like a good idea to resize the L2 cache
accordingly, but maybe this is out of scope for now as well? The purpose
of the current series is just to provide an option to automatically
calculate the needed L2 cache size for covering the entire image,
instead of using an external script to do that and feed the output to
l2-cache-size.
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.
You mean with QDict? I'll look into that now. But already sent v5 before
reading this email.
Also, a meta-comment: Leonid, would you mind sending plain text emails
instead of HTML-only?
Sure, Kevin! Sorry, I thought Thunderbird as configured here is sending
plain text. I was wrong. Now it should be fine. Thanks for the remark.
Leonid.
Kevin
