diff mbox

[v3] qmp: return err msg when powerdown a vm when it isn't in running state

Message ID 1450753771-5666-1-git-send-email-qhjin_dev@163.com
State New
Headers show

Commit Message

qhjindev Dec. 22, 2015, 3:09 a.m. UTC
When send system_powerdown to QMP when the vm isn't in RUN_STATE_RUNNING,
it will be ignored by system. So reply a err msg with the situation.

Signed-off-by: Qinghua Jin <qhjin_dev@163.com>
---

This is an update of the patch as per Fam Zheng's suggestion:
  http://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03949.html

 qmp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Prasad Pandit Dec. 22, 2015, 6:54 a.m. UTC | #1
+-- On Tue, 22 Dec 2015, Qinghua Jin wrote --+
| -void qmp_system_powerdown(Error **erp)
| +void qmp_system_powerdown(Error **errp)
|  {
| +    if (!runstate_check(RUN_STATE_RUNNING)) {
| +        error_setg(errp,
| +                   "Can not powerdown virtual machine as it is not running");
| +        return;
| +    }
|      qemu_system_powerdown_request();
|  }

Looks okay.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Markus Armbruster Jan. 18, 2016, 1:50 p.m. UTC | #2
Qinghua Jin <qhjin_dev@163.com> writes:

> When send system_powerdown to QMP when the vm isn't in RUN_STATE_RUNNING,
> it will be ignored by system. So reply a err msg with the situation.
>
> Signed-off-by: Qinghua Jin <qhjin_dev@163.com>
> ---
>
> This is an update of the patch as per Fam Zheng's suggestion:
>   http://lists.gnu.org/archive/html/qemu-devel/2015-12/msg03949.html
>
>  qmp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/qmp.c b/qmp.c
> index 0a1fa19..384df56 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -114,8 +114,13 @@ void qmp_system_reset(Error **errp)
>      qemu_system_reset_request();
>  }
>  
> -void qmp_system_powerdown(Error **erp)
> +void qmp_system_powerdown(Error **errp)
>  {
> +    if (!runstate_check(RUN_STATE_RUNNING)) {
> +        error_setg(errp,
> +                   "Can not powerdown virtual machine as it is not running");
> +        return;
> +    }
>      qemu_system_powerdown_request();
>  }

This is the handler for QMP command system_powerdown.  The patch makes
the QMP command fail in run states other than RUN_STATE_RUNNING.

There are other callers: the HMP command handler hmp_system_powerdown(),
GTK's gd_menu_powerdown(), Cocoa's powerDownQEMU().  They all ignore the
error.  Why is that appropriate?

qemu_system_powerdown_request() sets powerdown_requested = 1.  This
makes the next qemu_powerdown_requested() return 1.  Gets called only by
main_loop_should_exit().  I can't quite see how "it will be ignored by
system" when we're in RUN_STATE_RUNNING.  Can you explain?
diff mbox

Patch

diff --git a/qmp.c b/qmp.c
index 0a1fa19..384df56 100644
--- a/qmp.c
+++ b/qmp.c
@@ -114,8 +114,13 @@  void qmp_system_reset(Error **errp)
     qemu_system_reset_request();
 }
 
-void qmp_system_powerdown(Error **erp)
+void qmp_system_powerdown(Error **errp)
 {
+    if (!runstate_check(RUN_STATE_RUNNING)) {
+        error_setg(errp,
+                   "Can not powerdown virtual machine as it is not running");
+        return;
+    }
     qemu_system_powerdown_request();
 }