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

[Kevin Wolf]

Am 12.03.2018 um 12:50 hat Daniel P. Berrangé geschrieben:
> 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.

Oh, thanks, I didn't know that one! That makes it an offical API that
the QAPI generators must keep working, so it should be fine to use. I'll
give it a try then.

Kevin