On 09/20/2013 05:54 AM, Kevin Wolf wrote: > For examples see the changes to qmp-commands.hx. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > blockdev.c | 57 ++++++++++++ > qapi-schema.json | 270 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 59 ++++++++++++ > 3 files changed, 386 insertions(+) >
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsBase',
> + 'data': { 'driver': 'str',
> + '*id': 'str',
> + '*discard': 'BlockdevDiscardOptions',
> + '*cache': 'BlockdevCacheOptions',
> + '*aio': 'BlockdevAIOOptions',
Is the double space intentional? Harmless, but inconsistent.
> + '*rerror': 'BlockdevOnError',
> + '*werror': 'BlockdevOnError',
> + '*throttling': 'BlockdevThrottlingOptions',
> + '*read-only': 'bool' } }
...
> +##
> +# @BlockdevOptionsVVFAT
> +#
> +# Driver specific block device options for the vvfat protocol.
> +#
> +# @dir: directory to be exported as FAT image
> +# @fat-type: #optional FAT type: 12, 16 or 32
> +# @floppy: #optional whether to export a floppy imae (true) or
> partitioned
> +# hard disk (false; default)
> +# @rw: #optional whether to allow write operations (default: false)
Why do we have 'read-only' in base, and 'rw' in vvfat? It feels like
the vvfat option is redundant.
> +##
> +# @BlockdevOptionsGenericCOWFormat
> +#
> +# Driver specific block device options for image format that have no option
> +# besides their data source and an optional backing file.
> +#
> +# @file: reference to or definition of the data source block device
Do you need to document this field...
> +# @backing: #optional reference to or definition of the backing file
> block
> +# device (if missing, taken from the image file content). It is
> +# allowed to pass an empty string here in order to disable the
> +# default backing file.
> +# @copy-on-read: #optional whether to enable copy on read for the device
> +# (default: false). Copy on read can only be used if the
> +# image is not read-only.
> +#
> +# Since: 1.7
> +##
> +{ 'type': 'BlockdevOptionsGenericCOWFormat',
> + 'base': 'BlockdevOptionsGenericFormat',
> + 'data': { '*backing': 'BlockdevRef',
> + '*copy-on-read': 'bool' } }
...given that it is only present by inheritence?
> +##
> +# @BlockdevOptionsQcow2
> +#
> +# Driver specific block device options for qcow2.
> +#
> +# @file: reference to or definition of the data source block device
> +#
> +# @backing: #optional reference to or definition of the backing file
> block
> +# device (if missing, taken from the image file content)
Same question. If you DO document inherited fields, you missed
@copy-on-read; if you DON'T document inherited fields, these two aren't
necessary (instead, you could just have some statement about: "In
addition to the fields documented in BlockdevOptionsGenericCOWFormat,
this struct includes:")
> +##
> +# @blockdev-add:
> +#
> +# Creates a new block device.
> +#
> +# @options: block device options for the new device
> +#
> +# Since: 1.7
> +##
> +{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
So deceptively simple :) All the real work is in the rest of the
(cool!) enhancements to the qapi type generator.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
