Re: [Qemu-devel] [PATCH 00/13] RFC: luks/encrypted qcow2 key management

[Maxim Levitsky]

On Thu, 2019-08-15 at 16:18 +0200, Markus Armbruster wrote:
> Kevin Wolf <address@hidden> writes:
> 
> > Am 14.08.2019 um 23:08 hat Eric Blake geschrieben:
> > > On 8/14/19 3:22 PM, Maxim Levitsky wrote:
> > > 
> > > > This is an issue that was raised today on IRC with Kevin Wolf. Really 
> > > > thanks
> > > > for the idea!
> > > > 
> > > > We agreed that this new qmp interface should take the same options as
> > > > blockdev-create does, however since we want to be able to edit the 
> > > > encryption
> > > > slots separately, this implies that we sort of need to allow this on 
> > > > creation
> > > > time as well.
> > > > 
> > > > Also the BlockdevCreateOptions is a union, which is specialized by the 
> > > > driver name
> > > > which is great for creation, but for update, the driver name is already 
> > > > known,
> > > > and thus the user should not be forced to pass it again.
> > > > However qmp doesn't seem to support union type guessing based on actual 
> > > > fields
> > > > given (this might not be desired either), which complicates this 
> > > > somewhat.
> > > 
> > > Does the idea of a union type with a default value for the discriminator
> > > help?  Maybe we have a discriminator which defaults to 'auto', and add a
> > > union branch 'auto':'any'.  During creation, if the "driver":"auto"
> > > branch is selected (usually implicitly by omitting "driver", but also
> > > possible explicitly), the creation attempt is rejected as invalid
> > > regardless of the contents of the remaining 'any'.  But during amend
> > > usage, if the 'auto' branch is selected, we then add in the proper
> > > "driver":"xyz" and reparse the QAPI object to determine if the remaining
> > > fields in 'any' still meet the specification for the required driver 
> > > branch.
> > > 
> > > This idea may still require some tweaks to the QAPI generator, but it's
> > > the best I can come up with for a way to parse an arbitrary JSON object
> > > with unknown validation, then reparse it again after adding more
> > > information that would constrain the parse differently.
> > 
> > Feels like this would be a lot of code just to allow the client to omit
> > passing a value that it knows anyway. If this were a human interface, I
> > could understand the desire to make commands less verbose, but for QMP I
> > honestly don't see the point when it's not trivial.
> 
> Seconded.


But what about my suggestion of adding something like:

{ 'union': 'BlockdevAmendOptions',

  'base': {
      'node-name':         'str' },

  'discriminator': { 'get_block_driver(node-name)' } ,

  'data': {
      'file':           'BlockdevCreateOptionsFile',
      'gluster':        'BlockdevCreateOptionsGluster',
      'luks':           'BlockdevCreateOptionsLUKS',
      'nfs':            'BlockdevCreateOptionsNfs',
      'parallels':      'BlockdevCreateOptionsParallels',
      'qcow':           'BlockdevCreateOptionsQcow',
      'qcow2':          'BlockdevCreateOptionsQcow2',
      'qed':            'BlockdevCreateOptionsQed',
      'rbd':            'BlockdevCreateOptionsRbd',
      'sheepdog':       'BlockdevCreateOptionsSheepdog',
      'ssh':            'BlockdevCreateOptionsSsh',
      'vdi':            'BlockdevCreateOptionsVdi',
      'vhdx':           'BlockdevCreateOptionsVhdx',
      'vmdk':           'BlockdevCreateOptionsVmdk',
      'vpc':            'BlockdevCreateOptionsVpc'
  } }


This shouldn't be hard to do IMHO.

Best regards,
        Maxim Levitsky