Re: [Qemu-devel] [RFC PATCH 01/10] block/qapi: Introduce BlockdevCreateOptions

[Kevin Wolf]

Am 16.01.2018 um 19:54 hat Eric Blake geschrieben:
> On 01/11/2018 01:52 PM, Kevin Wolf wrote:
> > This creates a BlockdevCreateOptions union type that will contain all of
> > the options for image creation. We'll start out with an empty struct
> > type BlockdevCreateDummy for all drivers.
> > 
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  qapi/block-core.json | 64 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 64 insertions(+)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index e94a6881b2..1749376c61 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3320,6 +3320,70 @@
> >  { 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
> >  
> >  ##
> > +# @BlockdevCreateDummy:
> > +#
> > +# FIXME To be removed. Only there to make the QAPI generator happy while 
> > we're
> > +# adding driver by driver. Leaving out union branches is not allowed.
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'struct': 'BlockdevCreateDummy', 'data': {}}
> 
> At one point, I had a patch that let you do:
> 
> 'data': { 'branch': {},
>           ... }
> 
> for the branches that didn't need to add any additional types to the
> flat union.  Hmm, looks like it is still sitting in my tree, unapplied;
> would it help if I revived that one?

I actually realised that this type won't go away because we have drivers
that don't support image creation. I'm not sure if I should prefer the
conciseness of this syntax or the documentation that comes with a full
type in this specific case. Though generally, I think allowing inline
definitions would be nice.

The infrastructure that I was actually looking for here was a union type
that doesn't accept all enum values, but only some. Maybe some kind of
sub-enum for the discriminator, so that I still get the usual
BLOCKDEV_DRIVER_* constants, but allow only some of them in the context
of the union.

> > +
> > +##
> > +# @BlockdevCreateOptions:
> > +#
> > +# Options for creating an image format on a given node.
> > +#
> > +# @driver           block driver to create the image format
> > +# @node             node to create the image format on
> 
> Any restrictions we want to document about the node (for example, it
> must not be in use by any backend device at the moment, particularly
> since creation may change the node's format)?

Hm... As long as we can get BLK_PERM_WRITE, we should be good, right?

By the way, I've moved this to the driver-specific options since because
protocol drivers don't need it.

> > +#
> > +# Since: 2.12
> > +##
> > +{ 'union': 'BlockdevCreateOptions',
> > +  'base': {
> > +      'driver':         'BlockdevDriver',
> > +      'node':           'BlockdevRef' },
> > +  'discriminator': 'driver',
> > +  'data': {
> > +      'blkdebug':       'BlockdevCreateDummy',
> 
> The QAPI itself looks sane for the future patches.

Kevin

signature.asc
Description: PGP signature