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

[Eric Blake]

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 <address@hidden>
> > ---
> >   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