Re: [Qemu-devel] [PATCH RESEND 1/1] qmp.c: system_wakeup: adding RUN_STATE_SUSPENDED check before proceeding

[Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>]

Ping

On 01/02/2018 03:06 PM, Daniel Henrique Barboza wrote:
> The qmp/hmp command 'system_wakeup' is simply a direct call to
> 'qemu_system_wakeup_request' from vl.c. This function verifies if
> runstate is SUSPENDED and if the wake up reason is valid before
> proceeding. However, no error or warning is thrown if any of those
> pre-requirements isn't met.
>
> This leads to situations such as the one described in
> https://github.com/open-power-host-os/qemu/issues/31, where one
> can induce the OS to be suspended by using pm-suspend (via
> dompmsuspend, for example) but for some reason the machine failed to
> go to the SUSPENDED runstate, staying at runstate RUNNING. The user
> then tries to wake up the guest using system_wakeup (or dompmwakeup),
> no error is thrown but nothing happened either because the wake up
> wasn't fired at all. In the end, the user is left with a guest that
> is dormant and believing that system_wakeup isn't working.
>
> This patch changes qmp_system_wakeup to make the runstate verification
> before proceeding to call qemu_system_wakeup_request, firing up
> an error message if the user tries to wake up a machine that
> isn't in SUSPENDED state. The change isn't made inside
> qemu_system_wakeup_request because it is used in migration,
> ACPI and others where this usage might be valid. This patch
> by no means fixes the situation described above, but it can direct
> the user/management closer to the real problem.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   hmp.c | 4 +++-
>   qmp.c | 5 +++++
>   2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hmp.c b/hmp.c
> index 35a7041824..2c2326a00e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1158,7 +1158,9 @@ void hmp_cont(Monitor *mon, const QDict *qdict)
>
>   void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
>   {
> -    qmp_system_wakeup(NULL);
> +    Error *err = NULL;
> +    qmp_system_wakeup(&err);
> +    hmp_handle_error(mon, &err);
>   }
>
>   void hmp_nmi(Monitor *mon, const QDict *qdict)
> diff --git a/qmp.c b/qmp.c
> index 52cfd2d81c..d0be43fa1a 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -201,6 +201,11 @@ void qmp_cont(Error **errp)
>
>   void qmp_system_wakeup(Error **errp)
>   {
> +    if (!runstate_check(RUN_STATE_SUSPENDED)) {
> +        error_setg(errp,
> +                   "Unable to wake up: guest is not in suspended state");
> +        return;
> +    }
>       qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
>   }
>