[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] acpi: Add emulated sleep button
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] acpi: Add emulated sleep button |
Date: |
Thu, 20 Jul 2017 07:05:50 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
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..
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature