Skip to contentSkip to conversation

qmp.c: system_wakeup: adding RUN_STATE_SUSPENDED check before proceeding

PATCH Superseded Daniel Henrique Barboza 2 participants 2 messages May 14, 2018 – May 15, 2018

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 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.

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.

CC Markus Armbruster <[email protected]>
CC Dr. David Alan Gilbert <[email protected]>
CC Eric Blake <[email protected]>

---

This is a v2 of a patch that got buried:

http://lists.gnu.org/archive/html/qemu-devel/2018-01/msg00092.html

The changes in this version was all in the wording of the commit, reflecting the understanding reached in that discusssion: silent failure should be interpreted as a bug 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.

 hmp.c | 4 +++-
 qmp.c | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)
hmp.c +3 -1
index 898e25f3e1..2c4c867cab 100644
@@ -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
index 25fdc9a5b2..9c1d0df1ab 100644
@@ -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
Reviewed-by on v2 1/1
#1 lore raw
Quoted 9 lines · Daniel Henrique Barboza

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 would be the ideal place to also mention...

Quoted 12 lines

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.

CC Markus Armbruster <[email protected]>
CC Dr. David Alan Gilbert <[email protected]>
CC Eric Blake <[email protected]>
Quoted 8 lines

---

This is a v2 of a patch that got buried:

http://lists.gnu.org/archive/html/qemu-devel/2018-01/msg00092.html

The changes in this version was all in the wording of the commit, reflecting the understanding reached in that discusssion:

...this text:

Quoted 6 lines

silent
failure should be interpreted as a bug 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.

With the commit message further improved,

Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org