One more thing...
Markus Armbruster <arm...@redhat.com> writes:
> Daniel Henrique Barboza <danielhb...@gmail.com> writes:
>
>> 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. There is no way for the caller to
>> differentiate between a successful wakeup or an error state caused
>> when trying to wake up a guest that wasn't suspended.
>>
>> This means that system_wakeup is silently failing, which can be
>> considered a bug. Adding error handling isn't an API break in this
>> case - applications that didn't check the result will remain broken,
>> the ones that check it will have a chance to deal with it.
>>
>> Adding to that, the commit before previous created a new QMP API called
>> query-current-machine, with a new flag called wakeup-suspend-support,
>> that indicates if the guest has the capability of waking up from suspended
>> state. Although such guest will never reach SUSPENDED state and erroring
>> it out in this scenario would suffice, it is more informative for the user
>> to differentiate between a failure because the guest isn't suspended versus
>> a failure because the guest does not have support for wake up at all.
>>
>> All this considered, this patch changes qmp_system_wakeup to check if
>> the guest is capable of waking up from suspend, and if it is suspended.
>> After this patch, this is the output of system_wakeup in a guest that
>> does not have wake-up from suspend support (ppc64):
>>
>> (qemu) system_wakeup
>> wake-up from suspend is not supported by this guest
>> (qemu)
>>
>> And this is the output of system_wakeup in a x86 guest that has the
>> support but isn't suspended:
>>
>> (qemu) system_wakeup
>> Unable to wake up: guest is not in suspended state
>> (qemu)
>>
>> Reported-by: Balamuruhan S <bal...@linux.vnet.ibm.com>
>> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
>> ---
>> hmp.c | 5 ++++-
>> hw/acpi/core.c | 4 +++-
>> hw/char/serial.c | 3 ++-
>> hw/input/ps2.c | 9 ++++++---
>> hw/timer/mc146818rtc.c | 3 ++-
>> include/sysemu/sysemu.h | 3 ++-
>> migration/migration.c | 7 +++++--
>> qapi/misc.json | 8 +++++++-
>> qmp.c | 13 ++++++++++++-
>> vl.c | 6 ++++--
>> 10 files changed, 47 insertions(+), 14 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index 7828f93a39..0f5d943413 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1220,7 +1220,10 @@ 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/hw/acpi/core.c b/hw/acpi/core.c
>> index 52e18d7810..a7073dd435 100644
>> --- a/hw/acpi/core.c
>> +++ b/hw/acpi/core.c
>> @@ -514,7 +514,9 @@ static uint32_t acpi_pm_tmr_get(ACPIREGS *ar)
>> static void acpi_pm_tmr_timer(void *opaque)
>> {
>> ACPIREGS *ar = opaque;
>> - qemu_system_wakeup_request(QEMU_WAKEUP_REASON_PMTIMER);
>> + Error *err = NULL;
>> +
>> + qemu_system_wakeup_request(QEMU_WAKEUP_REASON_PMTIMER, &err);
>> ar->tmr.update_sci(ar);
>> }
Leaks the error object when qemu_system_wakeup_request() fails.
If it cannot fail here, pass &error_abort.
If it can fail, but you want to ignore failure, pass NULL.
More of the same elsewhere.
[...]
