Re: [Qemu-devel] [PATCH 04/13] qapi: Formalize qcow2 encryption probing

[Eric Blake <eblake@redhat.com>]

On 05/10/2018 02:58 AM, Daniel P. Berrangé wrote:
> On Wed, May 09, 2018 at 06:55:21PM +0200, Max Reitz wrote:
>> Currently, you can give no encryption format for a qcow2 file while
>> still passing a key-secret.  That does not conform to the schema, so
>> this patch changes the schema to allow it.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   qapi/block-core.json | 44 ++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 40 insertions(+), 4 deletions(-)

>>   { 'union': 'BlockdevQcow2Encryption',
>> -  'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
>> +  'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
>>     'discriminator': 'format',
>> +  'default-variant': 'from-image',
>>     'data': { 'aes': 'QCryptoBlockOptionsQCow',
>> -            'luks': 'QCryptoBlockOptionsLUKS'} }
>> +            'luks': 'QCryptoBlockOptionsLUKS',
>> +            'from-image': 'BlockdevQcow2EncryptionSecret' } }
> 
> Bike-shedding on name, how about "auto" or "probe" ?

Either of those sounds nicer to me; 'auto' might be better in the 
context of creation (that way, we can state that creating a NEW image 
with x-blockdev-create maps 'auto' to 'luks'; while connecting to an 
EXISTING image maps 'auto' to either 'aes' or 'luks' as appropriate).

> 
> IIUC, this schema addition means the QAPI parser now allows
> 
>     encrypt.format=from-image,encrypt.key-secret=sec0,...other opts...

Yes.  You could, perhaps, add a special case on the command line parsing 
code to reject an explicit use of format=from-image, but the QMP should 
not reject an explicit discriminator.

Hmm, it plays in with my comment on 1/13 - should the QMP parser 
automatically set has_discriminator to true when it supplies the 
default?  If it does, you lose the ability to see whether the user 
supplied an explicit encrypt.format=from-image (or the equivalent when 
using QMP instead of the command line), if you wanted to enforce that 
the user MUST omit format when relying on the from-image variant.

I don't see a problem in allowing the user to explicitly specify the 
name of the default branch, but I _do_ think the patch is incomplete for 
not handling the new QCOW_CRYPT_FROM_IMAGE case and converting it as 
soon as possible back into one of the other two preferred enum values.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org