qmp.c: system_wakeup: adding RUN_STATE_SUSPENDED check before proceeding
PATCH v3 1/1 qmp.c: system_wakeup: adding RUN_STATE_SUSPENDED check before proceeding
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. Silent failures should be interpreted as bugs and there is no API break in fixing this behavior - applications that didn't check the result will remain broken, the ones that check will have a chance to deal with it.
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.
---
Changes in v3:
- improved commit message, as requested by Eric Blake
- added Eric's R-b
hmp.c | 4 +++- qmp.c | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)
hmp.c +3 -1
@@ -1156,7 +1156,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)
qmp.c +5 -0
@@ -205,6 +205,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 ); }
Signature · 1 line
2.14.3
Quoted 41 lines · * Daniel Henrique Barboza ([email protected]) wrote:
The qmp/hmp command '
system_wakeup' is simply a direct call to 'qemu_system_wakeup_request' fromvl.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. Silent failures should be interpreted as bugs and there is no API break in fixing this behavior - applications that didn't check the result will remain broken, the ones that check will have a chance to deal with it.
This patch changes
qmp_system_wakeupto make the runstate verification before proceeding to callqemu_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 insideqemu_system_wakeup_requestbecause it is used in migration, ACPI and others where this usage might be valid.Signed-off-by Daniel Henrique Barboza <[email protected]>Reviewed-by Eric Blake <[email protected]>CC Markus Armbruster <[email protected]>CC Dr. David Alan Gilbert <[email protected]>CC Eric Blake <[email protected]>---
Changes in v3:
- improved commit message, as requested by Eric Blake
- added Eric's R-bhmp.c | 4 +++- qmp.c | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-)hmp.c +0
-0
hmp.c void hmp_cont(Monitor *mon, const QDict *qdict) · line 1162
Quoted 8 lines
hmp.c +3
-1@@ -1156,7 +1156,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); }
Fine from HMP side:
Quoted 20 lines
hmp.c +0
-0void hmp_nmi (Monitor *mon,const QDict *qdict)qmp.c +5
-0@@ -205,6 +205,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 ); }Signature · 1 line
2.14.3
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK