On 12/19/2017 08:12 AM, Markus Armbruster wrote:
Daniel Henrique Barboza <danie...@linux.vnet.ibm.com> writes:
qmp_cpu is a nop that was created a while ago in commit 755f196898
("qapi: Convert the cpu command") for the sake of compatibility,
due to the existence of hmp_cpu.
The compatibility argument makes no sense to me, but it's the reason the
commit documented.
Reading the thread that added it back in 2011*, I am guessing that the
compatibility argument is based on the premise that every HMP command
should have a QMP counterpart. This also makes sense considering the
QAPI FAQ page I've mentioned a few days ago.
Today, there is no need or requirement to keep it as is. QMP is
meant to be as stateless as possible, thus any QMP command that
needs a specific monitor CPU setup should provide it in its
s/should/must/
arguments, instead of relying in the current QMP monitor state.
This patch flags qmp_cpu as deprecated in qemu-doc.texi,
qapi-schema.json and changes qmp_cpu body to show a deprecation
message if used.
Signed-off-by: Daniel Henrique Barboza <danie...@linux.vnet.ibm.com>
The "deprecation message" is actually a hard error. This is unusual.
When we deprecate an interface, we don't normally make its use fail, we
just warn. QMP doesn't provide a good way to warn, though.
Let's compare behavior:
0. Status quo do nothing and succeed
1. Your patch fail with GenericError
2. Your patch less error_setg() do nothing and succeed
3. Immediate removal fail with CommandNotFound
Does the difference between 1. and 3. matter at all? Or is it just
deprecation cargo cult?
Good point. If both the deprecation message and the removal will
result in error for the user ....
Here's my take on it. I'd prefer 3. Immediate removal. I'd also be
fine with 2. Your patch less error_setg(), followed by removal after the
customary deprecation period. 1. plus later removal just doesn't make
sense to me, but it's really no big deal, so if you guys want it...
It all goes down to whether we consider qmp_cpu, a QMP command that does
nothing since its creation and apparently no one ever used it, a
feature. If it's a
feature, then I vote for (2) and removal after 2 releases following the
regular
deprecation policy.
If we consider that applying the policy to qmp_cpu is more trouble than
it's worth,
I'll gladly resend this patch removing it entirely and rewording the
qemu-doc.texi
entry to mention that it was removed because it did nothing and it was of
no use for anyone.
danielhb
---
qapi-schema.json | 5 ++++-
qemu-doc.texi | 10 ++++++++++
qmp.c | 2 +-
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 18457954a8..b51b89e19b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1052,7 +1052,10 @@
#
# Since: 0.14.0
#
-# Notes: Do not use this command.
+# Notes: Do not use this command. It was deprecated in release 2.12.0 and will
+# be removed, with no replacement, at any time in the future. QMP
+# commands that rely on the current CPU monitor should specify it as a
+# parameter.
##
{ 'command': 'cpu', 'data': {'index': 'int'} }
diff --git a/qemu-doc.texi b/qemu-doc.texi
index f7317dfc66..09e6c5046a 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2516,6 +2516,16 @@ subsystem image.
The ``convert -s snapshot_id_or_name'' argument is obsoleted
by the ``convert -l snapshot_param'' argument instead.
+@section System emulator machine protocol commands
+
+@subsection qmp_cpu (since 2.12.0)
+
+The ``qmp_cpu'' command was implemented in release 0.14.0 as a functional
+no-op, remaining as such up to release 2.12.0 when it was deprecated and
+flagged for future removal. No replacement will be provided: any QMP command
+that uses the current CPU monitor should, instead, specify the CPU in its
+parameters.
+
@section System emulator human monitor commands
@subsection host_net_add (since 2.10.0)
diff --git a/qmp.c b/qmp.c
index e8c303116a..d8543d713d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -115,7 +115,7 @@ void qmp_system_powerdown(Error **erp)
void qmp_cpu(int64_t index, Error **errp)
{
- /* Just do nothing */
+ error_setg(errp, "qmp_cpu is deprecated, do not use this command");
}
void qmp_cpu_add(int64_t id, Error **errp)
