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

[Leonid Bloch]

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