Re: [Qemu-block] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing

2018-06-12 Thread Daniel P . Berrangé
On Mon, Jun 11, 2018 at 10:51:57PM +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 
> ---
>  qapi/block-core.json | 44 
>  block/qcow2.c|  3 +++
>  2 files changed, 43 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-block] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing

2018-06-11 Thread Max Reitz
On 2018-06-11 23:02, Eric Blake wrote:
> On 06/11/2018 03:51 PM, 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 
>> ---
>>   qapi/block-core.json | 44 
>>   block/qcow2.c    |  3 +++
>>   2 files changed, 43 insertions(+), 4 deletions(-)
> 
>> +##
>> +# @ImageInfoSpecificQCow2EncryptionNoInfo:
>> +#
>> +# Only used for the qcow2 encryption format "auto" in which the actual
>> +# encryption format is determined from the image header.  Therefore,
>> +# this encryption format will never be reported in
>> +# ImageInfoSpecificQCow2Encryption.
>> +#
>> +# Since: 3.0
>> +##
>> +{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
>> +  'data': { } }
> 
> Do we actually need this type, given Anton's work on making omitted
> branches automatically use an empty struct?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06836.html

Looks like no, we don't.  Great! :-)

I think I'll still keep part of the comment and move it down into the
description of ImageInfoSpecificQCow2Encryption so that anyone who's
wondering knows that this value won't appear.

Thanks for reviewing and pointing me at it,

Max



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [PATCH v2 04/10] qapi: Formalize qcow2 encryption probing

2018-06-11 Thread Eric Blake

On 06/11/2018 03:51 PM, 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 
---
  qapi/block-core.json | 44 
  block/qcow2.c|  3 +++
  2 files changed, 43 insertions(+), 4 deletions(-)



+##
+# @ImageInfoSpecificQCow2EncryptionNoInfo:
+#
+# Only used for the qcow2 encryption format "auto" in which the actual
+# encryption format is determined from the image header.  Therefore,
+# this encryption format will never be reported in
+# ImageInfoSpecificQCow2Encryption.
+#
+# Since: 3.0
+##
+{ 'struct': 'ImageInfoSpecificQCow2EncryptionNoInfo',
+  'data': { } }


Do we actually need this type, given Anton's work on making omitted 
branches automatically use an empty struct?


https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06836.html


+
  ##
  # @ImageInfoSpecificQCow2Encryption:
  #
@@ -53,7 +66,8 @@
'base': 'ImageInfoSpecificQCow2EncryptionBase',
'discriminator': 'format',
'data': { 'aes': 'QCryptoBlockInfoQCow',
-'luks': 'QCryptoBlockInfoLUKS' } }
+'luks': 'QCryptoBlockInfoLUKS',
+'auto': 'ImageInfoSpecificQCow2EncryptionNoInfo' } }


If Anton's patches go in first, you don't even have to change this type;

  
  ##

  # @ImageInfoSpecificQCow2:
@@ -2658,10 +2672,30 @@
  # @BlockdevQcow2EncryptionFormat:
  # @aes: AES-CBC with plain64 initialization venctors
  #
+# @auto:Determine the encryption format from the image
+#   header.  This only allows the use of the
+#   key-secret option.  (Since: 3.0)
+#
  # Since: 2.10
  ##
  { 'enum': 'BlockdevQcow2EncryptionFormat',
-  'data': [ 'aes', 'luks' ] }
+  'data': [ 'aes', 'luks', 'auto' ] }


the changed enum would be sufficient.


+
+##
+# @BlockdevQcow2EncryptionSecret:
+#
+# Allows specifying a key-secret without specifying the exact
+# encryption format, which is determined automatically from the image
+# header.
+#
+# @key-secret:  The ID of a QCryptoSecret object providing the
+#   decryption key.  Mandatory except when probing
+#   image for metadata only.
+#
+# Since: 3.0
+##
+{ 'struct': 'BlockdevQcow2EncryptionSecret',
+  'data': { '*key-secret': 'str' } }
  
  ##

  # @BlockdevQcow2Encryption:
@@ -2669,10 +2703,12 @@
  # Since: 2.10
  ##
  { 'union': 'BlockdevQcow2Encryption',
-  'base': { 'format': 'BlockdevQcow2EncryptionFormat' },
+  'base': { '*format': 'BlockdevQcow2EncryptionFormat' },
'discriminator': 'format',
+  'default-variant': 'auto',
'data': { 'aes': 'QCryptoBlockOptionsQCow',
-'luks': 'QCryptoBlockOptionsLUKS'} }
+'luks': 'QCryptoBlockOptionsLUKS',
+'auto': 'BlockdevQcow2EncryptionSecret' } }


This part is necessary, though, and looks correct.

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