[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 02/18] block: add ability to set a prefix for
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v5 02/18] block: add ability to set a prefix for opt names |
Date: |
Thu, 23 Feb 2017 11:28:39 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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
pgpwA0XkuwNcR.pgp
Description: PGP signature
[Qemu-block] [PATCH v5 03/18] qcow: document another weakness of qcow AES encryption, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 04/18] qcow: require image size to be > 1 for new images, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 05/18] iotests: skip 042 with qcow which dosn't support zero sized images, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 06/18] iotests: skip 048 with qcow which doesn't support resize, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 07/18] iotests: fix 097 when run with qcow, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 09/18] qcow: convert QCow to use QCryptoBlock for encryption, Daniel P. Berrange, 2017/02/21
[Qemu-block] [PATCH v5 08/18] qcow: make encrypt_sectors encrypt in place, Daniel P. Berrange, 2017/02/21