Re: [Qemu-block] [PATCH 3/6] luks: Support .bdrv_co_create

[Daniel P . Berrangé]

On Mon, Mar 12, 2018 at 12:47:21PM +0100, Kevin Wolf wrote:
> Am 12.03.2018 um 12:40 hat Daniel P. Berrangé geschrieben:
> > On Fri, Mar 09, 2018 at 06:27:10PM +0100, Kevin Wolf wrote:
> > > This adds the .bdrv_co_create driver callback to luks, which enables
> > > image creation over QMP.
> > > 
> > > Signed-off-by: Kevin Wolf <address@hidden>
> > > ---
> > >  qapi/block-core.json | 17 ++++++++++++++++-
> > >  block/crypto.c       | 34 ++++++++++++++++++++++++++++++++++
> > >  2 files changed, 50 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 524d51567a..07039bfe9c 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -3452,6 +3452,21 @@
> > >              '*preallocation':   'PreallocMode' } }
> > >  
> > >  ##
> > > +# @BlockdevCreateOptionsLUKS:
> > > +#
> > > +# Driver specific image creation options for LUKS.
> > > +#
> > > +# @file             Node to create the image format on
> > > +# @size             Size of the virtual disk in bytes
> > > +#
> > > +# Since: 2.12
> > > +##
> > > +{ 'struct': 'BlockdevCreateOptionsLUKS',
> > > +  'data': { 'file':             'BlockdevRef',
> > > +            'qcrypto':          'QCryptoBlockCreateOptionsLUKS',
> > > +            'size':             'size' } }
> > 
> > s/qcrypto/crypto/ in this field.
> > 
> > I do wonder about whether instead of embedding QCryptoBlockCreateOptionsLUKS
> > as a field, we could instead use QCryptoBlockCreateOptionsLUKS as the
> > base struct and just inherit from it to add file/size.
> > 
> > I'm not really fussed, but I wonder if you/Eric have any thoughts on the 
> > pros
> > or cons of inheritance vs embedding in this case.
> 
> I don't think QAPI has a way to embed it in JSON, but still provide a
> QCryptoBlockCreateOptionsLUKS C object. Originally I wanted to use
> inheritance, but instead of having the struct type reused, you get all
> field definitions copied. I guess you could then manually cast to the
> base type and it would still work, but it didn't really feel clean.

IIRC, QAPI generates a method

   $BASETYPE *qapi_$TYPENAME_base(const $TYPENAME *obj)

which just does the blind cast to the base type.

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