Re: [Qemu-devel] [RFC 2/5] acpi: introduce TYPE_ACPI_DEVICE_IF interface

[Igor Mammedov]

On Fri, 30 May 2014 09:19:04 -0600
Eric Blake <address@hidden> wrote:

> On 05/28/2014 08:57 AM, Igor Mammedov wrote:
> > ... it will be used to abstract generic ACPI bits from
> > device that implements ACPI interface.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/acpi/Makefile.objs                |    1 +
> >  hw/acpi/acpi_interface.c             |   15 ++++++++++++
> >  include/hw/acpi/acpi_dev_interface.h |   43 
> > ++++++++++++++++++++++++++++++++++
> >  qapi-schema.json                     |   18 ++++++++++++++
> >  4 files changed, 77 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/acpi/acpi_interface.c
> >  create mode 100644 include/hw/acpi/acpi_dev_interface.h
> > 
> 
> Just an interface review...
> 
> > +++ b/qapi-schema.json
> > @@ -4775,3 +4775,21 @@
> >  # Since: 2.1
> >  ##
> >  { 'command': 'query-memory-devices', 'returns': ['MemoryDeviceInfo'] }
> > +
> > +## @ACPIOSTInfo
> > +#
> > +# OSPM Status Indication for a device
> > +# possible values are as desribed in "_OST (OSPM Status Indication)"
> 
> s/desribed/described/
> 
> > +# chapter, decribing method _OST in ACPI spec.
> 
> s/decribing/describing/
> 
> > +#
> > +# @source_event: Arg0 - An Integer containing the source event
> > +#
> > +# @status_code: Arg1 – An Integer containing the status code
> 
> Why '-' (ASCII) vs. '–' (UTF-8) between the two lines? We generally
> stick to ASCII in this file.
> 
> s/_/-/ in new QAPI additions.
> 
> These docs do not match...
> 
> > +#
> > +# Since: 2.1
> > +##
> > +{ 'type': 'ACPIOSTInfo',
> > +  'data'  : { 'device': 'str',
> > +              'source': 'int',
> > +              'status': 'int',
> > +              'slot': 'int' } }
> 
> ...this type declaration.  I have no idea which one of the two is wrong.
What do you mean under wrong?

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


-- 
Regards,
  Igor