[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions |
Date: |
Thu, 21 Mar 2013 01:31:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130307 Thunderbird/17.0.3 |
On 03/21/13 00:45, Eric Blake wrote:
> On 03/20/2013 05:23 PM, Laszlo Ersek wrote:
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> ---
>> qapi-schema.json | 58
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
> No counterpart change to qmp-commands.hx showing a valid usage?
This is not a qmp command, but a productive (ie. by-design)
repurposement of the schema. Please see commit
eb7ee2cb qapi: introduce OptsVisitor
and the somewhat heated
- http://thread.gmane.org/gmane.comp.emulators.qemu/193702/focus=194579
- http://thread.gmane.org/gmane.comp.emulators.qemu/193702/focus=194585
>> +#
>> +# @oem_id: #optional OEM identifier (6 bytes)
>
> s/oem_id/oem-id/
>
> In general, new QMP interfaces should use '-', not '_'.
Indeed! I think this warrants a respin.
>> +#
>> +# @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.
Again (referring back to the links above), the schema here is structured
so that it accepts the same "-acpitable ..." command line options that
used to work before.
The parsing of that option pre-series, in acpi_table_add(), is "XXX
fixme: this function uses obsolete argument parsing interface". Since
I'm reworking that here, I think it's reasonable to fix that up as well.
The choice is between the traditional QemuOpts functions and
OptsVisitor. I'd like to benefit from the range validation built into
the latter, plus I'd like to demonstrate that OptsVisitor is usable.
(BTW OptsVisitor does support repeating option arguments, and it indeed
turns them into JSON lists, but utilizing that here would break the
current '-acpitable ...' format.)
Thanks!
Laszlo
- [Qemu-devel] [PATCH 00/11] build ACPI MADT for fw_cfg clients, Laszlo Ersek, 2013/03/20
- [Qemu-devel] [PATCH 01/11] strip some whitespace, Laszlo Ersek, 2013/03/20
- [Qemu-devel] [PATCH 02/11] change element type from "char" to "unsigned char" in ACPI table data, Laszlo Ersek, 2013/03/20
- [Qemu-devel] [PATCH 03/11] acpi_table_add(): report fatal errors through an internal Error object, Laszlo Ersek, 2013/03/20
- [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Laszlo Ersek, 2013/03/20
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Eric Blake, 2013/03/20
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions,
Laszlo Ersek <=
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Laszlo Ersek, 2013/03/21
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Paolo Bonzini, 2013/03/21
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Michael S. Tsirkin, 2013/03/21
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Laszlo Ersek, 2013/03/21
- Re: [Qemu-devel] [PATCH 04/11] qapi schema: add AcpiTableOptions, Paolo Bonzini, 2013/03/21
[Qemu-devel] [PATCH 05/11] acpi_table_add(): accept QemuOpts and parse it with OptsVisitor, Laszlo Ersek, 2013/03/20
[Qemu-devel] [PATCH 07/11] like acpi_table_install(), acpi_table_add() should propagate Errors, Laszlo Ersek, 2013/03/20
[Qemu-devel] [PATCH 06/11] acpi_table_add(): extract and reimplement internals, Laszlo Ersek, 2013/03/20
[Qemu-devel] [PATCH 08/11] extract/unify the constant 0xfee00000 as APIC_DEFAULT_ADDRESS, Laszlo Ersek, 2013/03/20
[Qemu-devel] [PATCH 10/11] pc_acpi_init(): don't bail as soon as failing to find default DSDT, Laszlo Ersek, 2013/03/20