Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]

On 03/20/2013 05:23 PM, Laszlo Ersek wrote:
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  qapi-schema.json |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++

No counterpart change to qmp-commands.hx showing a valid usage?

>  1 files changed, 58 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index fdaa9da..aae6767 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3442,3 +3442,61 @@
>  # Since: 1.5
>  ##
>  { 'command': 'query-tpm', 'returns': ['TPMInfo'] }
> +
> +##
> +# @AcpiTableOptions
> +#
> +# Specify an ACPI table on the command line to load.
> +#

> +#
> +# @oem_id: #optional OEM identifier (6 bytes)

s/oem_id/oem-id/

In general, new QMP interfaces should use '-', not '_'.

> +#
> +# @file: #optional colon (:) separated list of pathnames to load and
> +#        concatenate as table data. The resultant binary blob is expected to
> +#        have an ACPI table header. At least one file is required. This field
> +#        excludes @data.
> +#

Ewwww.  This should be '*file' : [ 'str' ] (that is, use a JSON array of
file names, not a single string).  If you have to reparse a JSON
argument to break it into parts, then you are using the wrong interface;
not to mention that I might (perversely) want to pass in a file name
that contains a colon as part of its name.

> +# @data: #optional colon (:) separated list of pathnames to load and
> +#        concatenate as table data. The resultant binary blob must not have an
> +#        ACPI table header. At least one file is required. This field excludes
> +#        @file.

Again, JSON array, not flat string.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]