The code to check command policy can see special feature flag
'deprecated' as command flag QCO_DEPRECATED. I want to make feature
flag 'unstable' visible there as well, so I can add policy for it.
To let me make it visible, add member @special_features (a bitset of
QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
through qmp_register_command(). Then replace "QCO_DEPRECATED in
@flags" by QAPI_DEPRECATED in @special_features", and drop
QCO_DEPRECATED.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
---
include/qapi/qmp/dispatch.h | 5 +++--
monitor/misc.c | 6 ++++--
qapi/qmp-dispatch.c | 2 +-
qapi/qmp-registry.c | 4 +++-
storage-daemon/qemu-storage-daemon.c | 3 ++-
scripts/qapi/commands.py | 9 ++++-----
6 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 0ce88200b9..1e4240fd0d 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -25,7 +25,6 @@ typedef enum QmpCommandOptions
QCO_ALLOW_OOB = (1U << 1),
QCO_ALLOW_PRECONFIG = (1U << 2),
QCO_COROUTINE = (1U << 3),
- QCO_DEPRECATED = (1U << 4),
} QmpCommandOptions;
typedef struct QmpCommand
@@ -34,6 +33,7 @@ typedef struct QmpCommand
/* Runs in coroutine context if QCO_COROUTINE is set */
QmpCommandFunc *fn;
QmpCommandOptions options;
+ unsigned special_features;
QTAILQ_ENTRY(QmpCommand) node;
bool enabled;
const char *disable_reason;
@@ -42,7 +42,8 @@ typedef struct QmpCommand
typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
void qmp_register_command(QmpCommandList *cmds, const char *name,
- QmpCommandFunc *fn, QmpCommandOptions options);
+ QmpCommandFunc *fn, QmpCommandOptions options,
+ unsigned special_features);
const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
const char *name);
void qmp_disable_command(QmpCommandList *cmds, const char *name,
diff --git a/monitor/misc.c b/monitor/misc.c
index 3556b177f6..c2d227a07c 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -230,11 +230,13 @@ static void monitor_init_qmp_commands(void)
qmp_init_marshal(&qmp_commands);
- qmp_register_command(&qmp_commands, "device_add", qmp_device_add, 0);
+ qmp_register_command(&qmp_commands, "device_add",
+ qmp_device_add, 0, 0);
QTAILQ_INIT(&qmp_cap_negotiation_commands);
qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
}
/* Set the current CPU defined by the user. Callers must hold BQL. */
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 7e943a0af5..8cca18c891 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
"The command %s has not been found", command);
goto out;
}
- if (cmd->options & QCO_DEPRECATED) {
+ if (cmd->special_features & 1u << QAPI_DEPRECATED) {
switch (compat_policy.deprecated_input) {
case COMPAT_POLICY_INPUT_ACCEPT:
break;
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index f78c064aae..485bc5e6fc 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -16,7 +16,8 @@
#include "qapi/qmp/dispatch.h"
void qmp_register_command(QmpCommandList *cmds, const char *name,
- QmpCommandFunc *fn, QmpCommandOptions options)
+ QmpCommandFunc *fn, QmpCommandOptions options,
+ unsigned special_features)
{
QmpCommand *cmd = g_malloc0(sizeof(*cmd));
@@ -27,6 +28,7 @@ void qmp_register_command(QmpCommandList *cmds, const char *name,
cmd->fn = fn;
cmd->enabled = true;
cmd->options = options;
+ cmd->special_features = special_features;
QTAILQ_INSERT_TAIL(cmds, cmd, node);
}
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 10a1a33761..52cf17e8ac 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -146,7 +146,8 @@ static void init_qmp_commands(void)
QTAILQ_INIT(&qmp_cap_negotiation_commands);
qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
}
static int getopt_set_loc(int argc, char **argv, const char *optstring,
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index c8a975528f..21001bbd6b 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -26,6 +26,7 @@
QAPISchemaModularCVisitor,
build_params,
ifcontext,
+ gen_special_features,
)
from .schema import (
QAPISchema,
@@ -217,9 +218,6 @@ def gen_register_command(name: str,
coroutine: bool) -> str:
options = []
- if 'deprecated' in [f.name for f in features]:
- options += ['QCO_DEPRECATED']
-
if not success_response:
options += ['QCO_NO_SUCCESS_RESP']
if allow_oob:
@@ -231,10 +229,11 @@ def gen_register_command(name: str,
ret = mcgen('''
qmp_register_command(cmds, "%(name)s",
- qmp_marshal_%(c_name)s, %(opts)s);
+ qmp_marshal_%(c_name)s, %(opts)s, %(feats)s);
''',
name=name, c_name=c_name(name),
- opts=' | '.join(options) or 0)
+ opts=' | '.join(options) or 0,
+ feats=gen_special_features(features))
return ret
--
2.31.1
Markus Armbruster <armbru@redhat.com> wrote: > The code to check command policy can see special feature flag > 'deprecated' as command flag QCO_DEPRECATED. I want to make feature > flag 'unstable' visible there as well, so I can add policy for it. > > To let me make it visible, add member @special_features (a bitset of > QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it > through qmp_register_command(). Then replace "QCO_DEPRECATED in > @flags" by QAPI_DEPRECATED in @special_features", and drop > QCO_DEPRECATED. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Acked-by: John Snow <jsnow@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
On Thu, Oct 28, 2021 at 12:25:17PM +0200, Markus Armbruster wrote:
> The code to check command policy can see special feature flag
> 'deprecated' as command flag QCO_DEPRECATED. I want to make feature
> flag 'unstable' visible there as well, so I can add policy for it.
>
> To let me make it visible, add member @special_features (a bitset of
> QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
> through qmp_register_command(). Then replace "QCO_DEPRECATED in
> @flags" by QAPI_DEPRECATED in @special_features", and drop
> QCO_DEPRECATED.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Acked-by: John Snow <jsnow@redhat.com>
> ---
> +++ b/qapi/qmp-dispatch.c
> @@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
> "The command %s has not been found", command);
> goto out;
> }
> - if (cmd->options & QCO_DEPRECATED) {
> + if (cmd->special_features & 1u << QAPI_DEPRECATED) {
I admit having to check the C operator precedence table when reading
this (<< is higher than &); if writing it myself, I would probably
have used explicit () to avoid reviewer confusion, but what you have
is correct. (After grepping for ' & 1.*<<' and ' & (1.*<<', it looks
like authors using explicit precedence happens more often, but that
there are other instances in the code base relying on implicit
precedence.)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 10/29/21 17:28, Eric Blake wrote:
> On Thu, Oct 28, 2021 at 12:25:17PM +0200, Markus Armbruster wrote:
>> The code to check command policy can see special feature flag
>> 'deprecated' as command flag QCO_DEPRECATED. I want to make feature
>> flag 'unstable' visible there as well, so I can add policy for it.
>>
>> To let me make it visible, add member @special_features (a bitset of
>> QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
>> through qmp_register_command(). Then replace "QCO_DEPRECATED in
>> @flags" by QAPI_DEPRECATED in @special_features", and drop
>> QCO_DEPRECATED.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Acked-by: John Snow <jsnow@redhat.com>
>> ---
>
>> +++ b/qapi/qmp-dispatch.c
>> @@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
>> "The command %s has not been found", command);
>> goto out;
>> }
>> - if (cmd->options & QCO_DEPRECATED) {
>> + if (cmd->special_features & 1u << QAPI_DEPRECATED) {
>
> I admit having to check the C operator precedence table when reading
This doesn't seem a good use of (y)our time. Using a pair of parenthesis
is simpler.
I expect in a not far future that compilers emit a warning for this.
> this (<< is higher than &); if writing it myself, I would probably
> have used explicit () to avoid reviewer confusion, but what you have
> is correct. (After grepping for ' & 1.*<<' and ' & (1.*<<', it looks
> like authors using explicit precedence happens more often, but that
> there are other instances in the code base relying on implicit
> precedence.)
$ git grep -E ' & [0-9a-zA-Z_]+ <<'
hw/dma/pl330.c:997: if (~ch->parent->inten & ch->parent->ev_status &
1 << ev_id) {
hw/dma/xlnx-zynq-devcfg.c:198: if (s->regs[R_LOCK] & 1 << i) {
hw/intc/grlib_irqmp.c:144: if (s->broadcast & 1 << irq) {
hw/net/fsl_etsec/rings.c:491: if (etsec->regs[RSTAT].value & 1 << (23
- ring_nbr)) {
hw/net/virtio-net.c:748: (n->host_features & 1ULL <<
VIRTIO_NET_F_MTU)) {
hw/pci-host/mv64361.c:812: if ((ch & 0xff << i) && !(val
& 0xff << i)) {
hw/pci-host/mv64361.c:858: if (s->gpp_int_level && !(val & 0xff
<< b)) {
hw/ssi/xilinx_spi.c:123: qemu_set_irq(s->cs_lines[i],
!(~s->regs[R_SPISSR] & 1 << i));
hw/ssi/xilinx_spips.c:441: r[idx[!d]] |= x[idx[d]] & 1 <<
bit[d] ? 1 << bit[!d] : 0;
target/s390x/cpu_features.c:56: if (init[i] & 1ULL << j) {
tests/qtest/bios-tables-test.c:209: if (!(val & 1UL << 20 /*
HW_REDUCED_ACPI */)) {
Not that many.
© 2016 - 2026 Red Hat, Inc.