[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] acpi: Add emulated sleep button
From: |
Stefan Fritsch |
Subject: |
Re: [Qemu-devel] [PATCH] acpi: Add emulated sleep button |
Date: |
Thu, 20 Jul 2017 14:27:59 +0200 (CEST) |
User-agent: |
Alpine 2.11 (DEB 23 2013-08-11) |
On Thu, 20 Jul 2017, Eric Blake wrote:
> On 07/20/2017 04:31 AM, Stefan Fritsch wrote:
> > From: Stefan Fritsch <address@hidden>
> >
> > Add an ACPI sleep button and QMP/HMP commands to trigger it. A sleep
> > button is a so called "fixed hardware feature", which makes it more
> > suitable for putting the system to sleep than a laptop lid, for example.
> >
> > The sleep button is disabled by default (Bit 5 in the FACP flags
> > register set and no button "device" present in SSDT/DSDT). Clearing said
> > bit enables it as a fixed feature device.
> >
> > Signed-off-by: Stefan Fritsch <address@hidden>
> > ---
>
> Just an interface review:
>
> > +++ b/qapi-schema.json
> > @@ -2314,6 +2314,18 @@
> > { 'command': 'system_reset' }
> >
> > ##
> > +# @system_sleep:
> > +#
> > +# Requests that a guest perform a ACPI sleep transition by pushing a
> > virtual
>
> s/a /an /
>
> > +# sleep button.
> > +#
> > +# Notes: A guest may or may not respond to this command. This command
> > +# returning does not indicate that a guest has accepted the request
> > or
> > +# that it has gone to sleep.
>
> Missing a 'Since: 2.11' line.
>
> > +##
> > +{ 'command': 'system_sleep' }
>
> Please name this 'system-sleep' (new QMP commands should favor '-' over
> '_'). Or at least document in the commit message that you are
> intentionally trying to match spelling of older existing system_*
> commands. I wonder if it might be better to have a single system-acpi
> command that takes an enum of which action (sleep, powerdown, ...) to
> attempt, rather than a proliferation of commands per action..
I would prefer to match the spelling of older existing system_* commands
and will put that in the commit message. But weould be fine with the other
solutions, too.