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

[Kevin Wolf]

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.

Kevin