[Qemu-devel] [PATCH v1 1/1] qmp: remove qmp_cpu

[Qemu-devel] [PATCH v1 1/1] qmp: remove qmp_cpu

[Daniel Henrique Barboza]

'qmp_cpu' was implemented in commit 755f196898 ("qapi: Convert the cpu
command") as a functional no-op, a QMP call that does nothing and
return success. The idea, apparently, was to provide a counterpart
for the HMP 'hmp_cpu' command, introduced in the same commit.

After 6 years of its creation, qmp_cpu remains a functional no-op
that does nothing, having no value for any caller/user. A proposal
was sent to implement qmp_cpu like hmp_cpu works, but it was denied
[1]. The reason is that QMP must be as stateless as possible and a
function that changes its state (the current CPU monitor in the case
of qmp_cpu) goes against it. Any QMP command that needs a specific
monitor CPU setup must provide it in its arguments, instead of relying
in the current QMP monitor state.

After discussions that happened in [2] it was decided that a command
that does nothing since its birth, no one uses for anything and will
not be implemented, should be deprecated and erased. Given that we will
*not* provide any replacement for qmp_cpu and we believe that there
is no user relying on it, there is no point in adding a deprecation
delay for it.

So, this patch nukes qmp_cpu from QEMU code, removing both its blank
implementation in qmp.c and its doc in qapi-schema.json.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg02283.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03696.html

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: Daniel P. Berrange <berrange@redhat.com>
---
 qapi-schema.json | 11 -----------
 qmp.c            |  5 -----
 2 files changed, 16 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 18457954a8..5c29bcd27f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1046,17 +1046,6 @@
 { 'command': 'system_powerdown' }
 
 ##
-# @cpu:
-#
-# This command is a nop that is only provided for the purposes of compatibility.
-#
-# Since: 0.14.0
-#
-# Notes: Do not use this command.
-##
-{ 'command': 'cpu', 'data': {'index': 'int'} }
-
-##
 # @cpu-add:
 #
 # Adds CPU with specified ID
diff --git a/qmp.c b/qmp.c
index e8c303116a..52cfd2d81c 100644
--- a/qmp.c
+++ b/qmp.c
@@ -113,11 +113,6 @@ void qmp_system_powerdown(Error **erp)
     qemu_system_powerdown_request();
 }
 
-void qmp_cpu(int64_t index, Error **errp)
-{
-    /* Just do nothing */
-}
-
 void qmp_cpu_add(int64_t id, Error **errp)
 {
     MachineClass *mc;
-- 
2.13.6

Re: [Qemu-devel] [PATCH v1 1/1] qmp: remove qmp_cpu

[Daniel P. Berrange]

On Wed, Dec 20, 2017 at 08:23:04AM -0200, Daniel Henrique Barboza wrote:
> 'qmp_cpu' was implemented in commit 755f196898 ("qapi: Convert the cpu
> command") as a functional no-op, a QMP call that does nothing and
> return success. The idea, apparently, was to provide a counterpart
> for the HMP 'hmp_cpu' command, introduced in the same commit.
> 
> After 6 years of its creation, qmp_cpu remains a functional no-op
> that does nothing, having no value for any caller/user. A proposal
> was sent to implement qmp_cpu like hmp_cpu works, but it was denied
> [1]. The reason is that QMP must be as stateless as possible and a
> function that changes its state (the current CPU monitor in the case
> of qmp_cpu) goes against it. Any QMP command that needs a specific
> monitor CPU setup must provide it in its arguments, instead of relying
> in the current QMP monitor state.
> 
> After discussions that happened in [2] it was decided that a command
> that does nothing since its birth, no one uses for anything and will
> not be implemented, should be deprecated and erased. Given that we will
> *not* provide any replacement for qmp_cpu and we believe that there
> is no user relying on it, there is no point in adding a deprecation
> delay for it.
> 
> So, this patch nukes qmp_cpu from QEMU code, removing both its blank
> implementation in qmp.c and its doc in qapi-schema.json.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg02283.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03696.html
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Daniel P. Berrange <berrange@redhat.com>
> ---
>  qapi-schema.json | 11 -----------
>  qmp.c            |  5 -----
>  2 files changed, 16 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 18457954a8..5c29bcd27f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1046,17 +1046,6 @@
>  { 'command': 'system_powerdown' }
>  
>  ##
> -# @cpu:
> -#
> -# This command is a nop that is only provided for the purposes of compatibility.
> -#
> -# Since: 0.14.0
> -#
> -# Notes: Do not use this command.
> -##
> -{ 'command': 'cpu', 'data': {'index': 'int'} }
> -
> -##
>  # @cpu-add:
>  #
>  # Adds CPU with specified ID
> diff --git a/qmp.c b/qmp.c
> index e8c303116a..52cfd2d81c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -113,11 +113,6 @@ void qmp_system_powerdown(Error **erp)
>      qemu_system_powerdown_request();
>  }
>  
> -void qmp_cpu(int64_t index, Error **errp)
> -{
> -    /* Just do nothing */
> -}
> -
>  void qmp_cpu_add(int64_t id, Error **errp)
>  {
>      MachineClass *mc;
> -- 
> 2.13.6
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v1 1/1] qmp: remove qmp_cpu

[Markus Armbruster]

Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> writes:

> 'qmp_cpu' was implemented in commit 755f196898 ("qapi: Convert the cpu
> command") as a functional no-op, a QMP call that does nothing and
> return success. The idea, apparently, was to provide a counterpart
> for the HMP 'hmp_cpu' command, introduced in the same commit.
>
> After 6 years of its creation, qmp_cpu remains a functional no-op
> that does nothing, having no value for any caller/user. A proposal
> was sent to implement qmp_cpu like hmp_cpu works, but it was denied
> [1]. The reason is that QMP must be as stateless as possible and a
> function that changes its state (the current CPU monitor in the case
> of qmp_cpu) goes against it. Any QMP command that needs a specific
> monitor CPU setup must provide it in its arguments, instead of relying
> in the current QMP monitor state.
>
> After discussions that happened in [2] it was decided that a command
> that does nothing since its birth, no one uses for anything and will
> not be implemented, should be deprecated and erased. Given that we will
> *not* provide any replacement for qmp_cpu and we believe that there
> is no user relying on it, there is no point in adding a deprecation
> delay for it.
>
> So, this patch nukes qmp_cpu from QEMU code, removing both its blank
> implementation in qmp.c and its doc in qapi-schema.json.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg02283.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03696.html
>
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Daniel P. Berrange <berrange@redhat.com>

Lovely commit message.

I'll take this through my tree.  Thanks!

Re: [Qemu-devel] [PATCH v1 1/1] qmp: remove qmp_cpu

[Eric Blake]

On 12/20/2017 04:23 AM, Daniel Henrique Barboza wrote:
> 'qmp_cpu' was implemented in commit 755f196898 ("qapi: Convert the cpu
> command") as a functional no-op, a QMP call that does nothing and
> return success. The idea, apparently, was to provide a counterpart
> for the HMP 'hmp_cpu' command, introduced in the same commit.
> 
> After 6 years of its creation, qmp_cpu remains a functional no-op
> that does nothing, having no value for any caller/user. A proposal
> was sent to implement qmp_cpu like hmp_cpu works, but it was denied
> [1]. The reason is that QMP must be as stateless as possible and a
> function that changes its state (the current CPU monitor in the case
> of qmp_cpu) goes against it. Any QMP command that needs a specific
> monitor CPU setup must provide it in its arguments, instead of relying
> in the current QMP monitor state.

s/in/on/

> 
> After discussions that happened in [2] it was decided that a command
> that does nothing since its birth, no one uses for anything and will
> not be implemented, should be deprecated and erased. Given that we will
> *not* provide any replacement for qmp_cpu and we believe that there
> is no user relying on it, there is no point in adding a deprecation
> delay for it.
> 
> So, this patch nukes qmp_cpu from QEMU code, removing both its blank
> implementation in qmp.c and its doc in qapi-schema.json.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg02283.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03696.html
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Daniel P. Berrange <berrange@redhat.com>
> ---
>   qapi-schema.json | 11 -----------
>   qmp.c            |  5 -----
>   2 files changed, 16 deletions(-)

Took a while to iterate to this solution, but I love the end result.

Reviewed-by: Eric Blake <eblake@redhat.com>

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