Re: [Qemu-devel] [PATCH v5 02/18] block: add ability to set a prefix for opt names

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]

Am 22.02.2017 um 19:28 hat Eric Blake geschrieben:
> Using '.' would mean a layer of {} nesting on the wire, maybe as in:
> 
> { "driver": "qcow2", ..., "luks" : { "hash-alg": ... } }
> 
> but conceptually, I like that a bit better, as it consolidates all the
> luks-related options in one place, and may indeed make it possible to
> reuse the type rather than having two variants (one prefixed, one not,
> depending on whether it is standalone or qcow2).

Right, and this extra nesting to keep everything luks related in one
place is exactly what I wanted to achieve with it.

> I'm also looking later in your series (13/18), where you have:
> 
> 
> @@ -2344,7 +2348,8 @@
>              '*l2-cache-size': 'int',
>              '*refcount-cache-size': 'int',
>              '*cache-clean-interval': 'int',
> -            '*aes-key-secret': 'str' } }
> +            '*aes-key-secret': 'str',
> +            '*luks-key-secret': 'str' } }
> 
> 
> Uggh - we have two optional parameters, that must not both be present at
> once.  I'm wondering if we can instead do this (hmm, my patches for
> anonymous base/branches in a flat union haven't been taken yet, but you
> get the idea):
> 
> ...
> '*cache-clean-interval': 'int',
> '*encryption': 'Qcow2Encryption' } }
> 
> { 'enum': 'Qcow2EncryptionType': [ 'aes', 'luks' ] }
> { 'union': 'Qcow2Encryption', 'base': { 'type': 'Qcow2EncryptionType' },
>   'discriminator': 'type', 'data': {
>     'aes': { 'key-secret': 'str' },
>     'luks': { 'key-secret': 'str', '*hash-alg': ..., '*slot': 'int' } } }
> 
> so that you can only provide one encryption type, but once you have that
> type, you can then provide all the associated fields for that type.  So
> the QMP would look like:
> 
> { "driver": "qcow2", ..., "encryption" : { "type": "luks", "hash-alg":
> ... } }

That's actually even better, a more accurate description of the options
on the QAPI level. I like it.

> > Hence, I wanted to have separation between the legacy AES & LUKS
> > namespaces, to make it clear what applies to what scenario.
> 
> Again, I think the idea of a flat union, with the discriminator, makes
> it easier to enforce mutual exclusion, rather than having two top-level
> optional fields that cannot both be specified at once.  Maybe we should
> also consider making qapi flat unions support a default discriminator,
> so that the "type":"luks" can be omitted, but that's sugar.

With the default discriminator, I'm not sure if that's still "sugar" or
already "too much magic".

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]