| Message ID | 20211025052532.3859634-7-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Configurable policy for handling unstable interfaces | expand |
On 10/25/21 07:25, 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> > --- > 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); What about: typedef unsigned QapiFeatureMask; ? Otherwise: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Mon, Oct 25, 2021 at 1:25 AM 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> > --- > 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)) > Ah, you use the '0' return here. Alright then. > return ret > > > -- > 2.31.1 > > Python bits: Acked-by: John Snow <jsnow@redhat.com> C bits: "I believe in my heart that they probably work." (for this and previous patch.)
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 10/25/21 07:25, 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> >> --- >> 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); > > What about: > > typedef unsigned QapiFeatureMask; > > ? I think the name @special_features makes the connection to QapiSpecialFeature clear enough. > Otherwise: > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks!
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
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> --- 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(-)