Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG

[Eric Blake <eblake@redhat.com>]

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

On 06/13/2014 03:47 PM, Eric Blake wrote:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>>  docs/qmp/qmp-events.txt |   19 -------------------
>>  hw/watchdog/watchdog.c  |   23 +++++++----------------
>>  monitor.c               |    2 +-
>>  qapi-event.json         |   15 +++++++++++++++
>>  qapi-schema.json        |   24 ++++++++++++++++++++++++
>>  5 files changed, 47 insertions(+), 36 deletions(-)
>>

> 
>> +  'data': { 'action': 'WatchdogExpirationAction' } }
> 
> Hmm.  You've managed to create error.json in such a manner that it is
> not self-sufficient.  If some other file includes error.json, it must
> also define WatchdogExpirationAction or it will fail the generators.

s/error.json/qapi-event.json/

> 
>> +++ b/qapi-schema.json
> 
>> +##
>> +# @WatchdogExpirationAction
> 
> I think you will be better off to ensure that error.json is

and again qapi-event.json (not sure why I typed error.json).

> self-sufficient, perhaps by sticking any data type it references
> directly into common.json rather than qapi-schema.json, and having
> error.json include common.json.  (This is the first instance of
> referencing an external type, but other events later in the series have
> the same issue).

Oh weird! I've managed to run all four of
scripts/qapi-{visit,types,commands,event}.py directly on
qapi-event.json, and didn't get any complaints from the generator.  BUT,
the generated code is definitely different:

-void qapi_event_send_watchdog(WatchdogExpirationAction action,
+void qapi_event_send_watchdog(WatchdogExpirationAction * action,
                               Error **errp)

That is, when the enum type is known (because the parse was done on
qapi-schema.json), the WatchdogExpirationAction argument is treated as
an integer enum value; but when the enum type is unknown (because the
parse was done directly on an incomplete qapi-event.json), the generator
tries to treat it as a pointer to an otherwise unknown structure.
(Never mind the odd formatting of the space after the '*' - I believe
this pending patch fixes it:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02385.html).

This little exercise raises red flags for me - we probably ought to
enhance the code generators to error out instead of blindly act as if
unknown types are pointers.  But save it for another day - no need to
stall this series just to wait for a robustness improvement to the
generators.

Meanwhile, my suggestion of making qapi-event.json to be self-sufficient
is going to be a bit harder to test, but is still probably worth trying
(just moving common types into a common shared include).

-- 
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: 604 bytes --]