All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wenchao Xia <wenchaoqemu@gmail.com>
To: Eric Blake <eblake@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	qemu-devel@nongnu.org
Cc: mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH v3] qmp: add ACPI_DEVICE_OST event handling
Date: Sat, 21 Jun 2014 07:47:33 +0800	[thread overview]
Message-ID: <53A4C815.4040003@gmail.com> (raw)
In-Reply-To: <53A45753.6090505@redhat.com>

于 2014/6/20 23:46, Eric Blake 写道:
> On 06/20/2014 02:33 AM, Igor Mammedov wrote:
>> emits event when ACPI OSPM evaluates _OST method
>> of ACPI device.
>>
>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> ---
>> v2:
>>   - use new QAPI event infrastructure
>>        from rebased PCI tree on top of today's QMP pull request
>> ---
>>   hw/acpi/memory_hotplug.c |    7 ++++++-
>>   qapi-event.json          |   10 ++++++++++
>>   2 files changed, 16 insertions(+), 1 deletions(-)
>>
>
>> +++ b/qapi-event.json
>> @@ -304,3 +304,13 @@
>>   { 'event': 'QUORUM_REPORT_BAD',
>>     'data': { '*error': 'str', 'node-name': 'str',
>>               'sector-num': 'int', 'sector-count': 'int' } }
>> +
>> +##
>> +# @ACPI_DEVICE_OST
>> +#
>> +# Emitted when guest executes ACPI _OST method.
>> +#
>> +# @info: ACPIOSTInfo type as described in qapi-schema.json
>
> Needs 'Since: 2.1'.
>
   With above added,
Reviewed-by: Wenchao Xia <wenchaoqemu@gmail.com>

>> +##
>> +{ 'event': 'ACPI_DEVICE_OST',
>> +     'data': { 'info': 'ACPIOSTInfo' } }
>>
>
> Not your fault, as the problem already exists, but it's a bit awkward
> that qapi-event.json is not self-contained, and your patch is only
> making it worse.  qapi-event.json only makes sense when included by
> qapi-schema.json, when ideally it would be nice if it made sense if
> compiled in isolation. I already pointed this fact out on Wenchao's
> series that made events part of QAPI, but fixing it first requires
> teaching the code generators to flag places where a type is used without



> a pre-definition, so that we know which types have to be moved into a
> common include.  So don't let this comment hold up your patch.
>
   I think two issues should be addressed:
1 don't let generator guess the type if define is not found.
2 Support duplicated include, that is:

              qapi/types.json
                      |
       ---------------------------
       |                          |
qapi/qapi-event.json      qapi/block.json
              |                |
               qapi-schema-json

   Making sure above include doesn't generate two copy of types from
qapi/types.json.
   We can improve it later.

> If you add the Since line,
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

  reply	other threads:[~2014-06-20 23:48 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-20  8:33 [Qemu-devel] [PATCH v3] qmp: add ACPI_DEVICE_OST event handling Igor Mammedov
2014-06-20 15:46 ` Eric Blake
2014-06-20 23:47   ` Wenchao Xia [this message]
2014-06-23  9:52     ` Markus Armbruster
2014-06-23 15:56     ` Eric Blake
2014-06-22  5:58 ` Michael S. Tsirkin
2014-06-23 15:59   ` Eric Blake
2014-06-23 17:17     ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A4C815.4040003@gmail.com \
    --to=wenchaoqemu@gmail.com \
    --cc=eblake@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.