On 06/07/2016 09:51 AM, Daniel P. Berrange wrote:

>>
>> Missing documentation, but why do you need it, since it is identical to
>> QCryptoBlockInfoLUKSSlot in the previous patch?
>>

> 
> 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.

If we need to diverge in the future, then we can create the new type at
that time.  We intentionally made introspection hide type names, so that
we are not held hostage by type name changes, and so that splitting
types to add functionality to one but not all uses is safe.

> 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.

That may be an argument for making one type the base class for the
other, rather than a complete reimplementation.

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

Yep, just what I said :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org