On 07/26/2018 05:50 PM, Leonid Bloch wrote:
On 07/26/2018 05:42 PM, Kevin Wolf wrote:
Am 26.07.2018 um 14:24 hat Leonid Bloch geschrieben:
You mean with QDict? I'll look into that now. But already sent v5
before
reading this email.
Yes, with reading it from the QDict. (Or whatever the simplest way is
that results in the right external interface, but I suppose this is the
one.)
Well, there is a problem with that: I can easily isolate
l2-cache-size from QDict, check if it is "full", and if it is - do
whatever
is needed, and delete this option before parsing. But what if it is
"foo"?
It will not get deleted, and the regular QEMU_OPT_SIZE parsing error
will
appear, stating that l2-cache-size "expects a non-negative number..."
- no
word about that it can expect "full" as well. Now, one can try to modify
local_err->msg for this particular option, but this will require
substantial
additional logic. I think considering this, it would be easier to
stick with
a dedicated option, l2-cache-full.
Do you think there is a smarter way to parse the l2-cache-size
option, so it
would accept both size and "full", while handling errors correctly?
It seems
more elegant to have a single option, but the internal handling will
be more
elegant and simpler with two mutually exclusive options.
I think we can live with the suboptimal error message for a while. Once
qcow2 is QAPIfied, it should become easy to improve it. Let's not choose
a worse design (that stays forever) for a temporarily better error
message.
OK. I'll add a TODO then.
Another problem without a dedicated option, is that if l2-cache-size is
processed and deleted, it can not be read again when needed for
resizing. Without some *extremely* dirty tricks, that is.
Can QAPI be the solution? I've seen some examples, and it looks like the
qcow2 driver already uses QAPI to some extent - I mean all the qdict
stuff is from QAPI, no? Can you please point me to an example of how
QAPI can solve the issue of an option that can accept both a size and a
string?
Thanks,
Leonid.
By the way, the L2 cache resizes now on image resize. Will send the
changes
in v6. Thanks for the suggestion!
Sounds good!
Kevin
