Re: [Qemu-devel] [PATCH v5 0/8] Add metadata overlap checks

[Max Reitz <mreitz@redhat.com>]

On 2013-09-13 14:29, Eric Blake wrote:
> On 09/13/2013 04:23 AM, Max Reitz wrote:
>>> The more interesting part is that adding an option always needs thought
>>> because once it is exposed, it's an API that is set in stone. And I'm
>>> also not sure what the best command line and QMP representations of a
>>> bitmask like this are.
>> I'd personally add it to the runtime options of qcow2. In addition, I
>> propose we add a mechanism to generally amend runtime options at runtime
>> through QMP (if there isn't one already which I then am unaware of). I
>> don't see why we should just allow the kind of overlap checks performed
>> to be changed at runtime, but not, for instance, whether lazy refcounts
>> should be used (except the latter would be a bit harder to implement, I
>> guess).
> Indeed - I was asking more to spark conversation, and not to necessarily
> state that we need it (again, without benchmark numbers, it's hard to
> state whether there's enough timing difference for it to even matter
> that someone would WANT a runtime tuning).  IF we implement runtime
> tuning, we're stuck supporting it.
I have done some short tests here (I think just writing some GB of data 
to a qcow2 image on a ramdisk should suffice; maybe taking a couple of 
snapshots to increase the number of inactive L1 tables (although the 
active L2 tables should affect the results most anyway)) and I don't see 
a difference so far. I will continue those tests at home where I have 
more RAM.

>> About the representation: The discard behavior is basically a bitfield
>> already and gives us therefore one possible representation (which is,
>> just using a single boolean per structure, named something like
>> "overlap-check.active-l1" etc.). In QMP we could probably also use a
>> dict, but then again, this is a decision to be made when generally
>> allowing modification of the qcow2 runtime options through QMP (in my
>> opinion). And finally, we could obviously just use an integer to
>> represent the mask.
> Implement it only as a struct of bools.  A raw 'int' requires the caller
> to have too much internal knowledge of which bools map to which bit
> positions, and furthermore prevents you from ever changing bit positions.
>
>> I think, we should first take care of the command line interface and
>> about QMP later (that is, if you agree on generally allowing
>> modification of the qcow2 runtime options through QMP). There, we could
>> offer both one boolean per mask element and an integer option, probably
>> the boolean flags taking precedence.
> I'm fine if it is JUST a command-line parameter (all-or-nothing, turned
> on when you boot qemu, and not something we can be changing on the fly).
>   But if we ever do want live changing via QMP, do NOT expose it as a raw
> int, but only as named bools.
>
>> The flags are nice for users who want an "easily" comprehensible
>> interface, the masked integer is better for those who prefer a short
>> representation.
> Short representations that lock us into a particular implementation are bad.
>
Yes, that's true.


Max