Re: [Qemu-block] [PATCH v1 2/6] block: export LUKS specific data to qemu-img info

[Daniel P. Berrange]

On Tue, Jun 07, 2016 at 09:36:08AM -0600, Eric Blake wrote:
> On 06/07/2016 04:11 AM, Daniel P. Berrange wrote:
> > +++ b/qapi/block-core.json
> > @@ -74,6 +74,37 @@
> >        'extents': ['ImageInfo']
> >    } }
> >  
> > +
> > +{ 'struct': 'ImageInfoSpecificLUKSSlot',
> > +  'data': {'active': 'bool',
> > +           'iters': 'int',
> > +           'stripes': 'int',
> > +           'key-offset': 'int' } }
> 
> Missing documentation, but why do you need it, since it is identical to
> QCryptoBlockInfoLUKSSlot in the previous patch?
> 
> > +
> > +##
> > +# @ImageInfoSpecificLUKS:
> > +#
> > +# @cipher-alg: the cipher algorithm for data encryption
> > +# @cipher-mode: the cipher mode for data encryption
> > +# @ivgen-alg: the initialization vector generator
> > +# @ivgen-hash-alg: the initialization vector generator hash
> > +# @hash-alg: the master key hash algorithm
> > +#
> 
> Not all the members are documented, but isn't this identical to
> QCryptoBlockInfoLUKS in the previous patch?
> 
> > +# Since: 2.7
> > +##
> > +{ 'struct': 'ImageInfoSpecificLUKS',
> > +  'data': {
> > +      'cipher-alg': 'QCryptoCipherAlgorithm',
> > +      'cipher-mode': 'QCryptoCipherMode',
> > +      'ivgen-alg': 'QCryptoIVGenAlgorithm',
> > +      '*ivgen-hash-alg': 'QCryptoHashAlgorithm',
> > +      'hash-alg': 'QCryptoHashAlgorithm',
> > +      'payload-offset': 'int',
> > +      'master-key-iters': 'int',
> > +      'uuid': 'str',
> > +      'slots': [ 'ImageInfoSpecificLUKSSlot' ]
> > +  } }
> > +
> >  ##
> >  # @ImageInfoSpecific:
> >  #
> > @@ -85,7 +116,8 @@
> >  { 'union': 'ImageInfoSpecific',
> >    'data': {
> >        'qcow2': 'ImageInfoSpecificQCow2',
> > -      'vmdk': 'ImageInfoSpecificVmdk'
> > +      'vmdk': 'ImageInfoSpecificVmdk',
> > +      'luks': 'ImageInfoSpecificLUKS'
> 
> I guess the difference is whether you are giving the info on a LUKS
> image regardless of underlying storage, vs. on a qcow2 image with LUKS
> encryption. Still, can't we reuse the type, rather than duplicate it?

Essentially yes, and this is something I meant to mention in
the cover letter.

I wasn't really sure on the best approach to take here. I
could certainly re-use the QCrypto QAPI object by doing

{ 'union': 'ImageInfoSpecific',
  'data': {
      'qcow2': 'ImageInfoSpecificQCow2',
      'vmdk': 'ImageInfoSpecificVmdk',
      'luks': 'QCryptoBlockInfoLUKS',
  } }

I was not sure if that was a good idea or whether it is better
to have isolation between the crypto layer and block layer, as
safety net in case we need them to diverge. The main thing was
whether the data we report from the block driver will need to
include extra stuff not present in QCryptoBlockInfoLUKS, perhaps
related to the backing file/format.

I guess another option would be for ImageInfoSpecificLUKS
to sub-class QCryptoBlockInfoLUKS in that case.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|